-
Notifications
You must be signed in to change notification settings - Fork 1
Complete Authflow with WorkOS and SpaceTimeDB #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds monorepo scaffolding: formatting/git ignore, developer docs, SvelteKit + Tauri app and dashboard, Tauri Rust bootstrap and app-state/auth glue, SpacetimeDB-generated bindings (Rust + TS), OAuth/device-flow and JWT server logic, client-side crypto/vault stores, and assorted build/tooling configs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Desktop UI (Tauri Svelte)
participant TA as Tauri Rust App
participant Srv as Dashboard Server (WorkOS + JWT)
participant WorkOS as WorkOS OAuth
Note over UI,TA: Start device-auth flow (PKCE + device code)
UI->>TA: invoke("add_and_auth_new_user")
TA->>TA: create UserProfile, call AuthManager::start_new_auth_flow
TA->>Srv: (via device authorize endpoint) create device_code & user_code
Srv->>WorkOS: (if applicable) forward / produce verification URI
TA-->>UI: StartNewAuthFlowResponse { complete_uri, user_code }
UI->>WorkOS: user opens complete_uri and authenticates
WorkOS->>Srv: callback -> exchange code, validate, set cookies
Srv->>TA: desktopAuthManager marks session complete
TA->>TA: wait_for_user_auth resolves -> AuthorizationCompleteResponse
TA-->>UI: notify authenticated state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
🧹 Nitpick comments (19)
server/package.json (1)
7-8: Parameterize JWT key paths via environment variablesHardcoding the JWT key paths in the
devscript can lead to issues across different environments. Consider using environment variables (e.g.,JWT_PUB_KEY_PATHandJWT_PRIV_KEY_PATH) that default tojwt/public_key.pemandjwt/private_key.pem. This enhances portability and security.dashboard/src/routes/+page.svelte (1)
2-2: Addrel="noopener noreferrer"andtarget="_blank"to external linkExternal links should include
rel="noopener noreferrer"(andtarget="_blank"if opening in a new tab) to prevent potential security and performance issues:- <a href="https://svelte.dev/docs/kit"> + <a href="https://svelte.dev/docs/kit" target="_blank" rel="noopener noreferrer">dashboard/src/routes/auth/gate/+page.svelte (1)
1-3: Placeholder UI is minimal but functional
This simple loading state serves its purpose during the auth gate. Consider enhancing accessibility (e.g.,aria-busy="true") or adding a spinner/icon for a better UX in future iterations.dashboard/src/routes/dashboard/+page.svelte (1)
8-8: Consider limiting user data exposure in production.The complete JSON display of user data could potentially expose sensitive information. Consider either removing this in production or limiting it to specific fields for debugging purposes.
-<pre>{JSON.stringify(data.user, null, 2)}</pre> +<pre>{JSON.stringify({ firstName: data.user.firstName, email: data.user.email }, null, 2)}</pre>Or use a conditional display:
-<pre>{JSON.stringify(data.user, null, 2)}</pre> +{#if import.meta.env.DEV} + <pre>{JSON.stringify(data.user, null, 2)}</pre> +{/if}dashboard/.example.env (1)
1-12: Fix formatting issues flagged by dotenv-linter.The environment variable structure is well-organized, but there are several formatting issues that should be addressed for consistency:
Apply this diff to fix the formatting issues:
-WORKOS_CLIENT_ID="" -WORKOS_API_KEY="" +WORKOS_API_KEY="" +WORKOS_CLIENT_ID="" WORKOS_REDIRECT_URI="http://localhost:5173/auth/callback" WORKOS_SESSION_ID_COOKIE_NAME="workos_session_id" +JWT_COOKIE_NAME="axonotes_auth_token" +JWT_KEY_ID="" JWT_PRIVATE_KEY_BASE64="" JWT_PUBLIC_KEY_BASE64="" -JWT_KEY_ID="" -JWT_COOKIE_NAME="axonotes_auth_token" +PUBLIC_SPACETIME_MODULE_NAME="axonotes" PUBLIC_SPACETIME_WS="ws://localhost:3000" -PUBLIC_SPACETIME_MODULE_NAME="axonotes" +Alternatively, consider removing the empty quotes for a cleaner template:
-WORKOS_CLIENT_ID="" -WORKOS_API_KEY="" +WORKOS_API_KEY= +WORKOS_CLIENT_ID=🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
[warning] 2-2: [UnorderedKey] The WORKOS_API_KEY key should go before the WORKOS_CLIENT_ID key
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
[warning] 6-6: [QuoteCharacter] The value has quote characters (', ")
[warning] 7-7: [QuoteCharacter] The value has quote characters (', ")
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
[warning] 8-8: [UnorderedKey] The JWT_KEY_ID key should go before the JWT_PRIVATE_KEY_BASE64 key
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
[warning] 9-9: [UnorderedKey] The JWT_COOKIE_NAME key should go before the JWT_KEY_ID key
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
[warning] 12-12: [EndingBlankLine] No blank line at the end of the file
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
[warning] 12-12: [UnorderedKey] The PUBLIC_SPACETIME_MODULE_NAME key should go before the PUBLIC_SPACETIME_WS key
dashboard/src/hooks.server.ts (1)
5-21: Enhance type safety and cookie handling.The authentication logic is solid, but consider these improvements:
- Add type annotation for event.locals.user:
+import type { UserTokenPayload } from '$lib/server/jwt'; export const handle: Handle = async ({event, resolve}) => { const token = event.cookies.get(JWT_COOKIE_NAME); - event.locals.user = null; + event.locals.user = null as UserTokenPayload | null;
- Improve cookie deletion with proper options:
} else { - event.cookies.delete(JWT_COOKIE_NAME, {path: "/"}); + event.cookies.delete(JWT_COOKIE_NAME, { + path: "/", + httpOnly: true, + secure: true, + sameSite: 'lax' + }); }
- Consider adding error handling:
if (token) { + try { const userPayload = verifyToken(token); // ... rest of logic + } catch (error) { + console.error('Token verification error:', error); + event.cookies.delete(JWT_COOKIE_NAME, {path: "/"}); + } }scripts/generate-keys.ts (2)
56-58: Improve key ID generation for better uniqueness.The current date-based key ID may not provide sufficient uniqueness for production use.
Consider generating a more unique key ID:
console.log( - `JWT_KEY_ID="axonotes-key-${new Date().toISOString().slice(0, 10)}"`, + `JWT_KEY_ID="axonotes-key-${new Date().toISOString().slice(0, 10)}-${Math.random().toString(36).substring(2, 8)}"`, );Or use a more robust approach with crypto:
+import { randomBytes } from "node:crypto"; console.log( - `JWT_KEY_ID="axonotes-key-${new Date().toISOString().slice(0, 10)}"`, + `JWT_KEY_ID="axonotes-key-${new Date().toISOString().slice(0, 10)}-${randomBytes(4).toString('hex')}"`, );
36-45: Consider adding file permission security.For security, consider setting restrictive permissions on the generated key files.
Add this after writing the files:
+import { chmodSync } from "node:fs"; try { console.log(`\n💾 Saving PEM keys to the script's directory:`); console.log(` -> ${privateKeyPath}`); console.log(` -> ${publicKeyPath}`); writeFileSync(privateKeyPath, privateKey); writeFileSync(publicKeyPath, publicKey); + + // Set restrictive permissions (readable only by owner) + chmodSync(privateKeyPath, 0o600); + chmodSync(publicKeyPath, 0o644); + console.log(" ✅ Files created successfully."); } catch (error) {dashboard/src/app.html (1)
1-12: Consider adding essential meta tags for SEO and security.The HTML template is missing important meta tags for SEO, security, and social sharing.
<head> <meta charset="utf-8" /> + <meta name="description" content="Axonotes - Your secure note-taking platform" /> + <meta name="robots" content="index, follow" /> + <meta property="og:title" content="Axonotes" /> + <meta property="og:description" content="Your secure note-taking platform" /> + <meta property="og:type" content="website" /> <link rel="icon" href="%sveltekit.assets%/favicon.png" /> <meta name="viewport" content="width=device-width, initial-scale=1" /> %sveltekit.head% </head>dashboard/src/routes/+layout.svelte (2)
10-10: Consider header positioning and z-index implications.The absolute positioning with
z-20might cause layering issues with modal components or dropdowns that typically use higher z-index values.- <header class="absolute z-20 top-0 left-0 w-full flex justify-end items-center p-2"> + <header class="absolute z-40 top-0 left-0 w-full flex justify-end items-center p-2">
20-20: Optimize footer year calculation.The year calculation runs on every render. Consider moving it to a computed value or using a more efficient approach.
+<script lang="ts"> + import '../app.css'; + import LightSwitch from "$lib/components/LightSwitch.svelte"; + import AuthButton from "$lib/components/AuthButton.svelte"; + + let {children} = $props(); + const currentYear = new Date().getFullYear(); +</script> // In the footer: - © {new Date().getFullYear()} Axonotes. All rights reserved. + © {currentYear} Axonotes. All rights reserved.dashboard/src/lib/components/LightSwitch.svelte (1)
27-37: Consider synchronizing the head script with the effect logic.The head script and the effect both access localStorage and set the data-mode attribute. Consider extracting this logic to a shared function to maintain consistency.
Extract the mode logic:
<script lang="ts"> import {Moon, Sun} from "@lucide/svelte"; + const setMode = (mode: string) => { + document.documentElement.setAttribute('data-mode', mode); + try { + localStorage.setItem('mode', mode); + } catch (error) { + console.warn('Unable to save mode to localStorage:', error); + } + }; + + const getMode = (): string => { + try { + return localStorage.getItem('mode') || 'light'; + } catch (error) { + console.warn('Unable to load mode from localStorage:', error); + return 'light'; + } + }; let checked = $state(false); $effect(() => { - const mode = localStorage.getItem('mode') || 'light'; + const mode = getMode(); checked = mode === 'dark'; });dashboard/src/lib/server/jwt.ts (2)
60-62: Consider more secure error logging for production.The error logging might expose sensitive information in production environments. Consider using a more structured logging approach that omits sensitive details.
- console.error("JWT Verification failed:", error); + console.error("JWT Verification failed"); + // In production, log to monitoring service with appropriate context
40-46: Consider validating the payload structure before signing.While TypeScript provides compile-time type safety, runtime validation would ensure the payload contains all required fields before generating the token.
export function generateToken(payload: UserTokenPayload): string { + // Validate required fields + if (!payload.sub || !payload.iss || !payload.email) { + throw new Error("Invalid payload: missing required fields"); + } return jwt.sign(payload, privateKey, { algorithm: ALGORITHM, expiresIn: EXPIRES_IN, keyid: keyId, }); }dashboard/src/routes/auth/setup/+page.svelte (1)
57-62: Provide more specific error messages for debugging.The generic error message makes it difficult to diagnose issues. Consider preserving the actual error information while still being user-friendly.
if (!response.ok) { - throw new Error("Failed to save keys to the server."); + const errorText = await response.text(); + throw new Error(`Failed to save keys: ${response.status} ${errorText}`); } } catch (err) { - errorMessage = "An unexpected error occurred. Please try again."; + errorMessage = err instanceof Error && err.message.includes("Failed to save keys") + ? "Failed to save your encryption keys. Please try again." + : "An unexpected error occurred. Please try again."; console.error(err);dashboard/src/routes/auth/gate/+page.server.ts (1)
88-90: Use the identity index for better performance.Instead of converting the iterator to an array and using
find(), you can use the identity index directly.-const currentUser = Array.from(connection.db.user.iter()).find((user) => - user.identity.isEqual(connection!.identity!), -); +const currentUser = connection.db.user.identity.find(connection.identity!);dashboard/src/lib/client/crypto.ts (1)
62-88: Add key length validation for AES-256.The function assumes a 256-bit key but doesn't validate the input. Consider adding validation to ensure the key is exactly 32 bytes.
export async function encryptWithAes(data: string, key: Uint8Array) { + if (key.length !== 32) { + throw new Error('AES-256 requires a 32-byte (256-bit) key'); + } + const iv = window.crypto.getRandomValues(new Uint8Array(12)); const encoder = new TextEncoder(); const encodedData = encoder.encode(data);server/src/lib.rs (2)
1-1: Remove unused importTableThe
Tableimport is not used in this file.-use spacetimedb::{reducer, table, Identity, ReducerContext, Table}; +use spacetimedb::{reducer, table, Identity, ReducerContext};
38-49: Simplify logic by removing empty if blockThe empty if block adds no value and reduces readability.
pub fn client_connected(ctx: &ReducerContext) { - if let Some(_user) = ctx.db.user().identity().find(ctx.sender) { - } else { + if ctx.db.user().identity().find(ctx.sender).is_none() { ctx.db.user().insert(User { identity: ctx.sender, public_key: None, encrypted_backup_key: None, encrypted_private_key: None, argon_salt: None, }); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
dashboard/bun.lockis excluded by!**/*.lockdashboard/static/favicon.pngis excluded by!**/*.pngscripts/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
dashboard/.example.env(1 hunks)dashboard/.gitignore(1 hunks)dashboard/.npmrc(1 hunks)dashboard/.prettierignore(1 hunks)dashboard/.prettierrc(1 hunks)dashboard/README.md(1 hunks)dashboard/axonotes-theme.css(1 hunks)dashboard/eslint.config.js(1 hunks)dashboard/package.json(1 hunks)dashboard/src/app.css(1 hunks)dashboard/src/app.d.ts(1 hunks)dashboard/src/app.html(1 hunks)dashboard/src/hooks.server.ts(1 hunks)dashboard/src/lib/client/crypto.ts(1 hunks)dashboard/src/lib/components/AuthButton.svelte(1 hunks)dashboard/src/lib/components/LightSwitch.svelte(1 hunks)dashboard/src/lib/index.ts(1 hunks)dashboard/src/lib/module_bindings/client_connected_reducer.ts(1 hunks)dashboard/src/lib/module_bindings/index.ts(1 hunks)dashboard/src/lib/module_bindings/set_encryption_reducer.ts(1 hunks)dashboard/src/lib/module_bindings/user_table.ts(1 hunks)dashboard/src/lib/module_bindings/user_type.ts(1 hunks)dashboard/src/lib/server/jwt.ts(1 hunks)dashboard/src/lib/server/workos.ts(1 hunks)dashboard/src/routes/+layout.server.ts(1 hunks)dashboard/src/routes/+layout.svelte(1 hunks)dashboard/src/routes/+page.svelte(1 hunks)dashboard/src/routes/api/user/setup-keys/+server.ts(1 hunks)dashboard/src/routes/auth/callback/+server.ts(1 hunks)dashboard/src/routes/auth/gate/+page.server.ts(1 hunks)dashboard/src/routes/auth/gate/+page.svelte(1 hunks)dashboard/src/routes/auth/login/+server.ts(1 hunks)dashboard/src/routes/auth/logout/+server.ts(1 hunks)dashboard/src/routes/auth/setup/+page.svelte(1 hunks)dashboard/src/routes/dashboard/+page.server.ts(1 hunks)dashboard/src/routes/dashboard/+page.svelte(1 hunks)dashboard/svelte.config.js(1 hunks)dashboard/tsconfig.json(1 hunks)dashboard/vite.config.ts(1 hunks)scripts/.gitignore(1 hunks)scripts/README.md(1 hunks)scripts/generate-keys.ts(1 hunks)scripts/package.json(1 hunks)scripts/tsconfig.json(1 hunks)server/package.json(1 hunks)server/src/lib.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (14)
dashboard/src/routes/+layout.server.ts (1)
dashboard/src/routes/dashboard/+page.server.ts (1)
load(3-11)
dashboard/src/routes/dashboard/+page.server.ts (1)
dashboard/src/routes/+layout.server.ts (1)
load(1-5)
dashboard/src/hooks.server.ts (1)
dashboard/src/lib/server/jwt.ts (1)
verifyToken(53-64)
dashboard/src/routes/auth/login/+server.ts (2)
dashboard/src/routes/auth/callback/+server.ts (1)
GET(7-54)dashboard/src/lib/server/workos.ts (2)
workos(5-5)workosClientId(6-6)
dashboard/src/app.d.ts (1)
dashboard/src/lib/server/jwt.ts (1)
UserTokenPayload(8-14)
dashboard/src/routes/auth/logout/+server.ts (1)
dashboard/src/lib/server/workos.ts (1)
workos(5-5)
dashboard/src/routes/api/user/setup-keys/+server.ts (1)
dashboard/src/lib/module_bindings/index.ts (2)
DbConnection(143-150)ErrorContext(155-155)
dashboard/src/lib/module_bindings/set_encryption_reducer.ts (3)
dashboard/src/lib/module_bindings/index.ts (1)
SetEncryption(38-38)dashboard/src/lib/module_bindings/client_connected_reducer.ts (3)
getTypeScriptAlgebraicType(44-47)serialize(49-51)deserialize(53-55)dashboard/src/lib/module_bindings/user_type.ts (3)
getTypeScriptAlgebraicType(49-57)serialize(59-61)deserialize(63-65)
dashboard/src/lib/module_bindings/user_type.ts (3)
dashboard/src/lib/module_bindings/index.ts (1)
User(46-46)dashboard/src/lib/module_bindings/client_connected_reducer.ts (3)
getTypeScriptAlgebraicType(44-47)serialize(49-51)deserialize(53-55)dashboard/src/lib/module_bindings/set_encryption_reducer.ts (3)
getTypeScriptAlgebraicType(49-56)serialize(58-60)deserialize(62-64)
dashboard/src/routes/auth/gate/+page.server.ts (1)
dashboard/src/lib/module_bindings/index.ts (3)
DbConnection(143-150)ErrorContext(155-155)user(136-138)
dashboard/src/lib/module_bindings/client_connected_reducer.ts (3)
dashboard/src/lib/module_bindings/index.ts (1)
ClientConnected(36-36)dashboard/src/lib/module_bindings/set_encryption_reducer.ts (3)
getTypeScriptAlgebraicType(49-56)serialize(58-60)deserialize(62-64)dashboard/src/lib/module_bindings/user_type.ts (3)
getTypeScriptAlgebraicType(49-57)serialize(59-61)deserialize(63-65)
server/src/lib.rs (2)
dashboard/src/lib/module_bindings/index.ts (2)
user(136-138)User(46-46)dashboard/src/lib/module_bindings/user_type.ts (1)
User(33-39)
dashboard/src/lib/module_bindings/user_table.ts (2)
dashboard/src/lib/module_bindings/index.ts (3)
UserTableHandle(42-42)User(46-46)EventContext(152-152)dashboard/src/lib/module_bindings/user_type.ts (1)
User(33-39)
dashboard/src/lib/module_bindings/index.ts (4)
dashboard/src/lib/module_bindings/user_type.ts (1)
User(33-39)dashboard/src/lib/module_bindings/client_connected_reducer.ts (1)
ClientConnected(34-34)dashboard/src/lib/module_bindings/set_encryption_reducer.ts (1)
SetEncryption(34-39)dashboard/src/lib/module_bindings/user_table.ts (1)
UserTableHandle(46-106)
🪛 dotenv-linter (3.3.0)
dashboard/.example.env
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
[warning] 2-2: [UnorderedKey] The WORKOS_API_KEY key should go before the WORKOS_CLIENT_ID key
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
[warning] 6-6: [QuoteCharacter] The value has quote characters (', ")
[warning] 7-7: [QuoteCharacter] The value has quote characters (', ")
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
[warning] 8-8: [UnorderedKey] The JWT_KEY_ID key should go before the JWT_PRIVATE_KEY_BASE64 key
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
[warning] 9-9: [UnorderedKey] The JWT_COOKIE_NAME key should go before the JWT_KEY_ID key
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
[warning] 12-12: [EndingBlankLine] No blank line at the end of the file
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
[warning] 12-12: [UnorderedKey] The PUBLIC_SPACETIME_MODULE_NAME key should go before the PUBLIC_SPACETIME_WS key
🪛 Biome (1.9.4)
dashboard/src/app.d.ts
[error] 16-17: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
dashboard/src/lib/module_bindings/client_connected_reducer.ts
[error] 34-34: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (39)
dashboard/src/lib/index.ts (1)
1-1: Approve placeholder for$libaliasThis index file correctly serves as a marker for modules imported via the
$libalias in SvelteKit.dashboard/src/routes/+page.svelte (1)
1-6: Approve initial landing page scaffoldThis static page serves as a suitable placeholder and fits into the routing hierarchy. It can be expanded with dynamic content in future iterations.
dashboard/.prettierignore (1)
1-6: Approve.prettierignorefor lock filesIgnoring package manager lock files prevents unnecessary formatting changes. This configuration is appropriate.
dashboard/.npmrc (1)
1-1: Verify presence ofenginesfield inpackage.jsonThe
engine-strict=truesetting enforces versions defined inpackage.json. Confirm that yourdashboard/package.jsonincludes anenginessection specifying the supported Node.js (and npm) versions; otherwise, this directive may have no effect or cause unexpected failures.dashboard/.prettierrc (1)
1-15: Prettier configuration looks solid
The rules enforce consistent formatting across Svelte and Tailwind CSS files. Plugins and overrides align with your tech stack.scripts/README.md (1)
11-14: Verify script entrypoint naming consistency
The README instructsbun run index.ts, but the provided script is namedgenerate-keys.ts. Ensureindex.tsexists as a wrapper or update the command to referencegenerate-keys.ts.dashboard/src/app.css (2)
1-4: Tailwind CSS import may be incomplete
Using@import 'tailwindcss';alone might not inject the base, components, and utilities layers. Confirm your PostCSS pipeline handles Tailwind directives, or switch to:@tailwind base; @tailwind components; @tailwind utilities;
6-8: Ensure PostCSS plugins support nesting and custom variants
The@sourceand@custom-variantat-rules require PostCSS plugins likepostcss-nestingand a Tailwind custom-variant plugin. Verify these are configured in yourpostcss.config.jsto prevent build errors.dashboard/svelte.config.js (1)
1-2: Adapter and preprocessing setup look correct
Using@sveltejs/adapter-autowithvitePreprocess()covers most environments out of the box.dashboard/src/routes/+layout.server.ts (1)
1-5: LGTM! Correct layout-level user data propagation.This implementation correctly provides user data from the server context to all child routes without enforcing authentication at the layout level, allowing both public and protected routes to coexist appropriately.
dashboard/src/routes/dashboard/+page.server.ts (1)
3-11: LGTM! Proper authentication guard implementation.This correctly implements route-level authentication by checking for user presence and redirecting unauthenticated users. The pattern follows SvelteKit best practices for protected routes.
dashboard/.gitignore (1)
1-24: LGTM! Comprehensive .gitignore configuration.This .gitignore file appropriately covers all necessary patterns for a SvelteKit project including dependencies, build outputs, deployment platforms, OS files, and environment variables while preserving example files.
scripts/package.json (1)
1-15: LGTM! Clean and appropriate configuration.The package.json configuration is well-structured for a scripts utility package. The use of Bun for the key generation script and TypeScript peer dependency is appropriate for the authentication system setup.
dashboard/README.md (1)
1-38: LGTM! Clear and helpful documentation.The README provides comprehensive instructions for SvelteKit development workflow and correctly mentions the adapter requirement for deployment.
dashboard/vite.config.ts (1)
1-21: LGTM! Well-configured for crypto operations.The Vite configuration correctly includes Node.js polyfills needed for client-side cryptographic operations. The TailwindCSS and SvelteKit plugins are properly configured.
dashboard/src/lib/components/AuthButton.svelte (1)
1-11:✅ Verification successful
Verify authentication state consistency.
Ensure that
page.data.useris consistently populated by the layout server and properly reflects the authentication state across all routes.
🏁 Script executed:
#!/bin/bash # Description: Verify authentication state handling in layout and server hooks # Expected: Find layout server file and hooks that populate user data # Check for layout server file that should populate page.data.user fd "+layout.server.ts" dashboard/src/routes # Check for server hooks that handle authentication fd "hooks.server.ts" dashboard/src # Check for JWT handling and user verification logic rg -A 10 "locals\.user" dashboard/srcLength of output: 3568
Authentication state propagation verified.
dashboard/src/hooks.server.tsassignsevent.locals.userfrom the JWT (ornullif invalid)dashboard/src/routes/+layout.server.tsreturns{ user: locals.user }, makingpage.data.useravailable in all routesNo further changes needed.
dashboard/src/routes/auth/login/+server.ts (1)
1-14: LGTM! OAuth flow initiation implemented correctly.The login route properly implements the OAuth 2.0 authorization code flow by generating the authorization URL with appropriate parameters and redirecting the user to WorkOS. The integration with the callback handler in
dashboard/src/routes/auth/callback/+server.tscompletes the authentication flow correctly.dashboard/src/routes/auth/logout/+server.ts (1)
5-22: Excellent logout implementation with proper security practices.The logout handler correctly implements secure logout by:
- Attempting to revoke the WorkOS session server-side
- Using appropriate error handling that logs failures but doesn't interrupt the logout flow
- Properly cleaning up both authentication cookies with correct path specification
- Redirecting to a safe location after logout
This ensures users are properly logged out even if the WorkOS session revocation fails.
dashboard/src/app.d.ts (1)
1-18: TypeScript declarations are correctly implemented.The
app.d.tsfile properly extends the SvelteKitAppnamespace to include user authentication state inevent.locals. TheLocals.userproperty correctly typed asUserTokenPayload | nullaligns with the authentication flow implementation.Regarding the static analysis hint about the empty export: This is a false positive. The empty export is necessary in TypeScript to make this file a module when it contains only type declarations and imports. Without it, TypeScript would treat this as a script file rather than a module, which could cause issues with the global namespace declaration.
🧰 Tools
🪛 Biome (1.9.4)
[error] 16-17: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
scripts/.gitignore (1)
1-35: Comprehensive and well-organized gitignore file.The gitignore patterns appropriately cover all common artifacts for a Node.js/TypeScript project including dependencies, build outputs, logs, environment files, caches, and IDE-specific files. The clear section organization makes it easy to maintain.
dashboard/tsconfig.json (1)
1-21: LGTM! Well-configured TypeScript setup for SvelteKit.The configuration follows SvelteKit best practices with appropriate strict typing, modern module resolution, and helpful documentation comments.
dashboard/eslint.config.js (1)
12-36: LGTM - Comprehensive ESLint configuration.The configuration properly integrates multiple ESLint plugins and handles Svelte-specific requirements well. The TypeScript integration and Prettier compatibility are correctly set up.
dashboard/src/routes/+layout.svelte (1)
9-22: LGTM - Well-structured layout component.The layout provides a clean, responsive structure with proper authentication controls and theme support. The use of Tailwind classes and Svelte's reactive rendering is appropriate.
scripts/tsconfig.json (1)
1-28: Well-configured TypeScript setup for scripts.The configuration appropriately targets modern JavaScript features with ESNext, enables strict type checking, and uses bundler resolution. The disabled stricter flags (unused locals/parameters) make sense for utility scripts where temporary variables may be common.
dashboard/src/lib/components/LightSwitch.svelte (2)
6-9: Good defensive programming with localStorage handling.The component properly handles potential localStorage access issues with try-catch blocks and provides sensible fallbacks.
44-45: Excellent accessibility implementation.The component includes proper
aria-labelattributes for screen readers, making the theme toggle accessible to users with disabilities.Also applies to: 53-54
dashboard/src/lib/module_bindings/client_connected_reducer.ts (1)
1-2: This is an autogenerated file - modifications should be made to the source generator.The file is correctly generated and follows the expected pattern for SpacetimeDB reducers.
dashboard/src/routes/auth/setup/+page.svelte (1)
48-48:❓ Verification inconclusive
Verify the salt object structure before accessing buffer property.
The code assumes
salthas abufferproperty. Ensure this is consistent with the return type ofcrypto.generateSalt().
🏁 Script executed:
#!/bin/bash # Description: Check the implementation of generateSalt to verify it returns an object with buffer property # Search for generateSalt implementation ast-grep --pattern 'function generateSalt($_) { $$$ }' # Also check with export variations ast-grep --pattern 'export function generateSalt($_) { $$$ }'Length of output: 124
I can’t locate any
generateSaltdefinition in the codebase, nor see howcryptois imported into+page.svelte. Let’s double-check:
🏁 Script executed:
#!/bin/bash set -euxo pipefail # 1. Find the +page.svelte file PAGE=$(fd --full-path '+page.svelte' dashboard/src/routes/auth/setup) # 2. Show how crypto is imported grep -n "import.*crypto" -n "$PAGE" # 3. Search everywhere for generateSalt definition rg -n "generateSalt" . # 4. Confirm crypto methods available rg -n "arrayBufferToBase64" .Length of output: 383
Verify
salt.bufferusage withcrypto.generateSalt()I wasn’t able to locate the
generateSaltimplementation to confirm that its return value has a.bufferproperty. Please double-check:
- Where and how
crypto.generateSalt()is defined or imported.- That its return type is a
BufferorUint8Array(both expose.buffer); if it’s anArrayBuffer, you may need to usesaltdirectly or convert it appropriately before callingcrypto.arrayBufferToBase64.No code changes here—just ensure the
saltstructure matches this usage.dashboard/src/routes/api/user/setup-keys/+server.ts (2)
79-101: Authentication and validation logic is well-implemented.The dual authentication check (locals.user and JWT token) provides robust security, and the input validation ensures all required encryption fields are present before proceeding.
30-73:❓ Verification inconclusive
Consider improving the connection lifecycle management.
The current approach relies on a timeout and manual cleanup, but the connection success callback immediately disconnects. Consider whether you need to wait for a reducer completion event or if the current approach might disconnect before the operation completes on the server side.
🏁 Script executed:
#!/bin/bash # Verify if SpacetimeDB reducers provide completion callbacks or if the current approach is sufficient rg -A 10 -B 5 "\.reducers\." --type tsLength of output: 1444
Verify reducer completion before disconnecting
I didn’t find any completion or acknowledgement callback on
conn.reducers.setEncryptionin the codebase, so it’s unclear whether the 100 ms timeout is sufficient to guarantee the operation has been applied server-side. Please confirm with the SpacetimeDB client API or documentation that:
- The
setEncryptionreducer call is synchronous or returns a promise/event you can await.- If not, consider exposing and waiting for a completion callback or promise instead of relying on a fixed timeout.
Locations to review:
- dashboard/src/routes/api/user/setup-keys/+server.ts (lines 30–73)
dashboard/axonotes-theme.css (1)
1-205: Excellent comprehensive theme implementation.The theme is well-structured with:
- Modern OKLCH color space for better color consistency
- Comprehensive semantic color palette (primary, secondary, tertiary, success, warning, error, surface)
- Proper contrast color definitions for accessibility
- Clean typography and spacing variables
- Support for both light and dark modes
The organization and naming conventions are consistent and maintainable.
dashboard/src/lib/module_bindings/user_type.ts (1)
33-65: Auto-generated type definition follows consistent patterns.The
Usertype definition correctly implements:
- Required
identityfield usingIdentitytype- Optional encryption-related string fields using
string | undefined- Proper algebraic type representation with
createOptionTypefor optional fields- Standard serialization/deserialization pattern consistent with other generated types
The structure aligns well with the
SetEncryptiontype and follows SpacetimeDB's code generation conventions.dashboard/src/lib/module_bindings/set_encryption_reducer.ts (1)
34-65: Correct implementation of SetEncryption type.The type definition properly implements:
- All four required string fields for encryption data
- Consistent algebraic type representation using
createStringType()(no optional types)- Standard serialization/deserialization pattern
- Perfect alignment with the API endpoint's payload validation
The required nature of these fields ensures the API endpoint's validation logic will work correctly.
dashboard/src/lib/module_bindings/user_table.ts (1)
1-107: Auto-generated file - no manual changes needed.This file is automatically generated by SpacetimeDB. Any modifications should be made to the source module instead.
dashboard/src/lib/client/crypto.ts (2)
33-59: Secure RSA key generation implementation.The RSA key generation uses appropriate parameters: 4096-bit keys, RSA-OAEP with SHA-256, and standard key export formats.
91-102: Secure password hashing with appropriate Argon2 parameters.The Argon2id implementation uses reasonable parameters for client-side hashing. The 128MB memory cost provides good protection against GPU attacks.
server/src/lib.rs (2)
3-13: Well-structured user table for encryption key managementThe
Usertable is properly designed with identity as the primary key and optional encryption fields, allowing for progressive user setup.
23-30:⚠️ Potential issueCritical: Missing identity field in user update
The struct update is missing the
identityfield, which is the primary key. This could cause runtime errors or data corruption.ctx.db.user().identity().update(User { + identity: user.identity, public_key: Some(public_key), encrypted_private_key: Some(encrypted_private_key), encrypted_backup_key: Some(encrypted_backup_key), argon_salt: Some(argon_salt), - ..user });Likely an incorrect or invalid review comment.
dashboard/src/lib/module_bindings/index.ts (1)
1-156: Auto-generated file - no manual edits neededThis file is automatically generated and correctly reflects the server-side module structure. Any issues should be addressed in the source module (
server/src/lib.rs) rather than here.
git-subtree-dir: SpacetimeDB git-subtree-split: 1fb0e10244656ca9681aedc6512017eabb72c68e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 42
🧹 Nitpick comments (61)
SpacetimeDB/.jenkins/Jenkinsfile.internal (1)
46-54: Normalize post-build status handling
Remove trailing semicolons for consistency and consider handlingunstableorabortedoutcomes to cover all build states.post { success { setBuildStatus("Build succeeded", "SUCCESS") } unstable { setBuildStatus("Build unstable", "FAILURE") } failure { setBuildStatus("Build failed", "FAILURE") } }SpacetimeDB/crates/auth/src/identity.rs (2)
52-52: Fix typo in comment.There's a typo referencing "SpacetimeIdentityClaims2" instead of "SpacetimeIdentityClaims".
-// The signature should be verified already, but further validation is needed to have a SpacetimeIdentityClaims2. +// The signature should be verified already, but further validation is needed to have a SpacetimeIdentityClaims.
76-89: Consider making validation limits configurable.The hardcoded 128-byte limits for issuer and subject are reasonable but might benefit from being configurable constants or configuration parameters for different deployment scenarios.
+const MAX_ISSUER_LENGTH: usize = 128; +const MAX_SUBJECT_LENGTH: usize = 128; + impl TryInto<SpacetimeIdentityClaims> for IncomingClaims { type Error = anyhow::Error; fn try_into(self) -> anyhow::Result<SpacetimeIdentityClaims> { // The issuer and subject must be less than 128 bytes. - if self.issuer.len() > 128 { + if self.issuer.len() > MAX_ISSUER_LENGTH { return Err(anyhow::anyhow!("Issuer too long: {:?}", self.issuer)); } - if self.subject.len() > 128 { + if self.subject.len() > MAX_SUBJECT_LENGTH { return Err(anyhow::anyhow!("Subject too long: {:?}", self.subject)); }SpacetimeDB/.github/workflows/tag-release.yml (1)
18-19: Remove trailing whitespace.Static analysis detected trailing spaces that should be removed for cleaner formatting.
username: ${{ vars.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_PASSWORD }} - - - name: Retag the image + + - name: Retag the image🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 18-18: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
README.md (2)
54-62: Fix nested list indentation to follow markdown standards.The nested list items are using 4-space indentation when markdownlint expects 2 spaces for consistency.
- **`/app`**: - - The Axonotes desktop application. - - Built with **[Tauri](https://tauri.app/)** (using **[SvelteKit](https://kit.svelte.dev/)** for the frontend). - - Provides the cross-platform user interface (Windows, macOS, Linux) and client-side logic. - - Handles offline-first capabilities and synchronization with the backend. + - The Axonotes desktop application. + - Built with **[Tauri](https://tauri.app/)** (using **[SvelteKit](https://kit.svelte.dev/)** for the frontend). + - Provides the cross-platform user interface (Windows, macOS, Linux) and client-side logic. + - Handles offline-first capabilities and synchronization with the backend. - **`/server`**: - - The backend logic and data modules running on **[SpaceTimeDB](https://spacetimedb.com/)**. - - Written in **Rust**. - - Manages real-time collaboration, data persistence, and the detailed version history system. + - The backend logic and data modules running on **[SpaceTimeDB](https://spacetimedb.com/)**. + - Written in **Rust**. + - Manages real-time collaboration, data persistence, and the detailed version history system.🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: Loose punctuation mark.
Context: ...ation with the backend. -/server: - The backend logic and data module...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
54-54: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
60-60: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
61-61: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
79-89: Apply consistent 2-space indentation for tech stack lists.Similar indentation fix needed for the tech stack section.
- **Client-side (Desktop App):** - - Framework: [Tauri](https://tauri.app/) - - UI: [SvelteKit](https://kit.svelte.dev/) - - Language: TypeScript, HTML, CSS + - Framework: [Tauri](https://tauri.app/) + - UI: [SvelteKit](https://kit.svelte.dev/) + - Language: TypeScript, HTML, CSS - **Backend & Real-time Database:** - - Platform: [SpaceTimeDB](https://spacetimedb.com/) - - Language: Rust + - Platform: [SpaceTimeDB](https://spacetimedb.com/) + - Language: Rust - **Key Features Powered by this Stack:** - - Cross-platform native-like experience - - Real-time collaboration - - Robust offline-first capabilities - - Incredibly detailed version history + - Cross-platform native-like experience + - Real-time collaboration + - Robust offline-first capabilities + - Incredibly detailed version history🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
79-79: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
80-80: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
81-81: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
83-83: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
84-84: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
87-87: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
88-88: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
89-89: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
SpacetimeDB/.github/workflows/discord-posts.yml (2)
16-23: Use official GitHub Action to install GH CLI
Manually adding apt sources and installingghis brittle and slows down the workflow. Consider replacing these steps with the officialcli/gh-actionfor a more reliable setup.For example:
- - name: Set up GitHub CLI - run: | - curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | sudo tee /usr/share/keyrings/githubcli-archive-keyring.gpg > /dev/null - sudo apt-get install -y apt-transport-https - echo "deb [arch=amd64 signed-by=/usr/share/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list - sudo apt-get update - sudo apt-get install gh + - name: Set up GitHub CLI + uses: cli/gh-action@v3 + with: + token: ${{ secrets.GITHUB_TOKEN }}
33-34: Quote$GITHUB_ENVin redirection
To avoid word-splitting or path issues, wrap the environment file path in quotes.- echo "CHECK_RESULT=${RESULT}" >> $GITHUB_ENV + echo "CHECK_RESULT=${RESULT}" >> "$GITHUB_ENV"DEVELOPING.md (1)
1-4: Remove conversational intro and frontmatter marker
A developer guide should start with the project title. The greeting and leading---can be removed for clarity.-Of course! Thank you for the corrections and additional details. Here is the updated developer setup guide incorporating your feedback. - --- -🧰 Tools
🪛 LanguageTool
[uncategorized] ~1-~1: Possible missing comma found.
Context: ...ls. Here is the updated developer setup guide incorporating your feedback. --- # Ax...(AI_HYDRA_LEO_MISSING_COMMA)
SpacetimeDB/README.md (4)
72-72: Add missing comma for clarity.The sentence would be clearer with a comma after "module".
-Instead of deploying a web or game server that sits in between your clients and your database, your clients connect directly to the database and execute your application logic inside the database itself. You can write all of your permission and authorization logic right inside your module just as you would in a normal server. +Instead of deploying a web or game server that sits in between your clients and your database, your clients connect directly to the database and execute your application logic inside the database itself. You can write all of your permission and authorization logic right inside your module, just as you would in a normal server.🧰 Tools
🪛 LanguageTool
[uncategorized] ~72-~72: Possible missing comma found.
Context: ...d authorization logic right inside your module just as you would in a normal server. ...(AI_HYDRA_LEO_MISSING_COMMA)
89-89: Fix grammatical construction."All of application state" is grammatically incorrect.
-This speed and latency is achieved by holding all of application state in memory, while persisting the data in a write-ahead-log (WAL) which is used to recover application state. +This speed and latency is achieved by holding all application state in memory, while persisting the data in a write-ahead-log (WAL) which is used to recover application state.🧰 Tools
🪛 LanguageTool
[grammar] ~89-~89: Consider using “all application” or “all of the application”.
Context: ...peed and latency is achieved by holding all of application state in memory, while persisting the d...(ALL_MOST_SOME_OF_NOUN)
99-99: Use proper noun form."Install script" should be "installation script" for grammatical correctness.
-Installing on macOS is as simple as running our install script. After that you can use the spacetime command to manage versions. +Installing on macOS is as simple as running our installation script. After that, you can use the spacetime command to manage versions.-Installing on Linux is as simple as running our install script. After that you can use the spacetime command to manage versions. +Installing on Linux is as simple as running our installation script. After that, you can use the spacetime command to manage versions.Also applies to: 107-107
🧰 Tools
🪛 LanguageTool
[grammar] ~99-~99: The word ‘install’ is not a noun.
Context: ...ng on macOS is as simple as running our install script. After that you can use the spac...(A_INSTALL)
[typographical] ~99-~99: Consider adding a comma here.
Context: ...s simple as running our install script. After that you can use the spacetime command to ma...(FOR_THAT_COMMA)
291-291: Use proper compound adjective form.When "open source" modifies a noun as a compound adjective, it should be hyphenated.
-SpacetimeDB is licensed under the BSL 1.1 license. This is not an open source or free software license, however, it converts to the AGPL v3.0 license with a linking exception after a few years. +SpacetimeDB is licensed under the BSL 1.1 license. This is not an open-source or free software license; however, it converts to the AGPL v3.0 license with a linking exception after a few years.🧰 Tools
🪛 LanguageTool
[uncategorized] ~291-~291: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...der the BSL 1.1 license. This is not an open source or free software license, however, it c...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~291-~291: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ... an open source or free software license, however, it converts to the AGPL v3.0 license wi...(HOWEVER_SENTENCE)
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Internal/ByteArrayComparer.cs (1)
39-47: Consider using a better hash function for improved distribution.The current polynomial rolling hash (hash = hash * 31 + b) can have poor distribution for certain byte patterns and may cause hash collisions.
Consider using a more robust hash function:
public int GetHashCode(byte[] obj) { - var hash = 17; - foreach (var b in obj) - { - hash = hash * 31 + b; - } - return hash; + if (obj.Length == 0) return 0; + + // Use FNV-1a hash for better distribution + const uint offsetBasis = 2166136261; + const uint prime = 16777619; + + uint hash = offsetBasis; + foreach (var b in obj) + { + hash ^= b; + hash *= prime; + } + return (int)hash; }SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/U256.cs (1)
52-66: Consider simplifying CompareTo implementation.The current implementation can be simplified using the comparison operators that are already defined.
public int CompareTo(U256 value) { - if (this < value) - { - return -1; - } - else if (this > value) - { - return 1; - } - else - { - return 0; - } + return this < value ? -1 : this > value ? 1 : 0; }SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/U128.cs (1)
54-68: Consider simplifying CompareTo implementation.The current implementation can be simplified using the comparison operators.
public int CompareTo(U128 value) { - if (this < value) - { - return -1; - } - else if (this > value) - { - return 1; - } - else - { - return 0; - } + return this < value ? -1 : this > value ? 1 : 0; }SpacetimeDB/.github/workflows/package.yml (2)
75-78: Improve branch extraction robustness.The current branch extraction could be more robust and clearer.
- - name: Extract branch name - shell: bash - run: echo "branch=${GITHUB_HEAD_REF:-${GITHUB_REF#refs/heads/}}" >> $GITHUB_OUTPUT - id: extract_branch + - name: Extract branch name + shell: bash + run: | + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + echo "branch=${{ github.head_ref }}" >> $GITHUB_OUTPUT + else + echo "branch=${GITHUB_REF#refs/heads/}" >> $GITHUB_OUTPUT + fi + id: extract_branch
20-21: Consider adding ARM musl build when GitHub Actions supports it.The commented-out ARM musl build is a known limitation. Consider adding a TODO or tracking issue for when this becomes available.
# FIXME: arm musl build. "JavaScript Actions in Alpine containers are only supported on x64 Linux runners" + # TODO: Track GitHub Actions support for ARM musl builds # - { name: aarch64 Linux musl, target: aarch64-unknown-linux-musl, runner: arm-runner }SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/I256.cs (1)
55-69: Consider reordering method definitions for better readability.The
CompareTo(I256 value)method uses comparison operators (<and>) that are defined later in the file. While this compiles correctly, it could be confusing to readers. Consider either reordering the methods or implementing this without relying on the operators.Alternative implementation that doesn't rely on operators:
public int CompareTo(I256 value) { - if (this < value) - { - return -1; - } - else if (this > value) - { - return 1; - } - else - { - return 0; - } + if (IsNegative(this) != IsNegative(value)) + { + return IsNegative(this) ? -1 : 1; + } + + int upperCmp = _upper.CompareTo(value._upper); + if (upperCmp != 0) + return upperCmp; + + return _lower.CompareTo(value._lower); }SpacetimeDB/Cargo.toml (1)
297-297: Verify OpenSSL vendoring necessity.The vendored OpenSSL dependency increases build times and binary size. Ensure this is still necessary for the target deployment environments.
Consider documenting when vendored OpenSSL is required vs. when system OpenSSL can be used:
# Vendor the openssl we rely on, rather than depend on a # potentially very old system version. # This can be disabled for environments with up-to-date OpenSSL # by removing the "vendored" feature. openssl = { version = "0.10", features = ["vendored"] }SpacetimeDB/crates/bench/README.md (7)
33-33: Format bare URL as markdown linkThe bare URL should be formatted as a proper markdown link for consistency.
Apply this diff:
-These also rely on the benchmarks-viewer application (https://github.com/clockworklabs/benchmarks-viewer). +These also rely on the [benchmarks-viewer application](https://github.com/clockworklabs/benchmarks-viewer).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
33-33: Bare URL used
null(MD034, no-bare-urls)
49-87: Add language identifier to fenced code blockThe fenced code block should specify a language for proper syntax highlighting.
Change line 49 from:
-``` +```text🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
49-49: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
95-95: Minor: Consider more concise phrasing"all of the" can be simplified to "all" for conciseness.
-Will run all of the `insert_bulk` benchmarks against the raw spacetime backend. +Will run all `insert_bulk` benchmarks against the raw spacetime backend.🧰 Tools
🪛 LanguageTool
[style] ~95-~95: Consider removing “of” to be more concise
Context: ... 'stdb_raw/.*/insert_bulk' ``` Will run all of theinsert_bulkbenchmarks against the ra...(ALL_OF_THE)
101-101: Use consistent terminology: "disk" instead of "disc"For consistency with modern computing terminology, use "disk" instead of "disc".
-Will run benchmarks involving unique primary keys against all databases, without writing to disc. +Will run benchmarks involving unique primary keys against all databases, without writing to disk.🧰 Tools
🪛 LanguageTool
[grammar] ~101-~101: The word ‘disc’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ... against all databases, without writing to disc. ## Workload Benches The workload ben...(VB_TO_NN_DT)
148-148: Fix heading hierarchyHeading levels should increment by one. This jumps from h2 to h4.
-#### Targeted benchmarks +### Targeted benchmarks🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
148-148: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4(MD001, heading-increment)
151-151: Fix heading hierarchy for Generic benchmarksFor consistency with the previous heading fix.
-#### Generic benchmarks +### Generic benchmarks🧰 Tools
🪛 LanguageTool
[grammar] ~151-~151: This phrase is duplicated. You should probably use “Generic benchmarks” only once.
Context: ...Put them inbenches/special.rs. #### Generic benchmarks Generic benchmarks live inbenches/generic.rs. These ben...(PHRASE_REPETITION)
160-163: Fix list indentationThe sub-items should use 2-space indentation instead of 4-space.
-- Add a new generic benchmark, you'll need to: -- Add a relevant method to the `BenchDatabase` trait. -- Implement it for each `BenchDatabase` implementation. - - [`SQLite`](src/sqlite.rs) will require you to write it as a SQL query and submit that to sqlite. - - [`SpacetimeRaw`](src/spacetime_raw.rs) will require you to execute the query against the Spacetime database backend directly. - - [`SpacetimeModule`](src/spacetime_module.rs) will require you to add a reducer to the [`benchmarks`](../../modules/benchmarks/src/lib.rs) crate, and then add logic to invoke your reducer with the correct arguments. +To add a new generic benchmark, you'll need to: +- Add a relevant method to the `BenchDatabase` trait. +- Implement it for each `BenchDatabase` implementation. + - [`SQLite`](src/sqlite.rs) will require you to write it as a SQL query and submit that to sqlite. + - [`SpacetimeRaw`](src/spacetime_raw.rs) will require you to execute the query against the Spacetime database backend directly. + - [`SpacetimeModule`](src/spacetime_module.rs) will require you to add a reducer to the [`benchmarks`](../../modules/benchmarks/src/lib.rs) crate, and then add logic to invoke your reducer with the correct arguments.🧰 Tools
🪛 LanguageTool
[style] ~162-~162: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...cetimeModule](src/spacetime_module.rs) will require you to add a reducer to the [b...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.17.2)
160-160: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
161-161: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
162-162: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#PublicTable.verified.cs (1)
70-71: Consider improving ToString() readabilityThe ToString() method is extremely long on a single line. While this is auto-generated code, consider formatting it across multiple lines for better readability in the code generator.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomStruct.verified.cs (2)
62-67: Improve hash code distribution by avoiding XOR.The current implementation uses XOR to combine hash codes, which can lead to poor distribution. For example, if both fields have the same hash code, the result is 0.
Consider using a better hash combination approach:
public override int GetHashCode() { - var ___hashIntField = IntField.GetHashCode(); - var ___hashStringField = StringField == null ? 0 : StringField.GetHashCode(); - return ___hashIntField ^ ___hashStringField; + unchecked + { + var hash = 17; + hash = hash * 31 + IntField.GetHashCode(); + hash = hash * 31 + (StringField?.GetHashCode() ?? 0); + return hash; + } }
80-92: Simplify using pattern matching.The type checking logic can be simplified using modern C# pattern matching.
public override bool Equals(object? that) { - if (that == null) - { - return false; - } - var that_ = that as CustomStruct?; - if (((object?)that_) == null) - { - return false; - } - return Equals(that_); + return that is CustomStruct other && Equals(other); }SpacetimeDB/crates/bench/benches/subscription.rs (2)
62-105: Consider using expect() for better error messages in benchmarks.While
unwrap()may be acceptable in benchmark code, usingexpect()with descriptive messages would make debugging easier if setup fails.- let raw = SpacetimeRaw::build(false).unwrap(); + let raw = SpacetimeRaw::build(false).expect("Failed to build SpacetimeRaw"); - let lhs = create_table_footprint(&raw.db).unwrap(); - let rhs = create_table_location(&raw.db).unwrap(); + let lhs = create_table_footprint(&raw.db).expect("Failed to create footprint table"); + let rhs = create_table_location(&raw.db).expect("Failed to create location table");
73-104: Extract magic numbers into named constants.The benchmark uses several hardcoded values that would be clearer as named constants.
Add constants at the module level:
const FOOTPRINT_ROW_COUNT: u64 = 1_000_000; const OWNER_MODULO: u64 = 1_000; const LOCATION_CHUNKS: u64 = 1_000; const ROWS_PER_CHUNK: u64 = 1_200; const FOOTPRINT_TYPE_COUNT: u8 = 4;Then use these constants throughout the code for better readability and maintainability.
SpacetimeDB/crates/bench/benches/index.rs (1)
114-114: Improve variable naming for clarity.The variable name
keys2is unclear. Consider a more descriptive name.- let keys2 = keys.iter().copied().sorted(); + let sorted_keys = keys.iter().copied().sorted();SpacetimeDB/crates/bench/src/lib.rs (2)
47-48: Use path join for better readability.Consider using the
joinmethod for path construction.- let mut bench_dot_spacetime = Path::new(env!("CARGO_MANIFEST_DIR")).to_path_buf(); - bench_dot_spacetime.push(".spacetime"); + let bench_dot_spacetime = Path::new(env!("CARGO_MANIFEST_DIR")).join(".spacetime");
87-95: Document why bulk update is conditional.The bulk update operation is only performed for
IndexStrategy::Unique0without explanation. Consider adding a comment explaining this constraint.+ // Bulk update requires a unique index on the first column to identify rows if index_strategy == IndexStrategy::Unique0 { db.update_bulk::<T>(&table_id, count)?;SpacetimeDB/crates/bench/benches/special.rs (2)
72-72: Fix grammatical errors in comments.- // Initialize outside the benchmark so the db is seed once, to avoid to enlarge the db + // Initialize outside the benchmark so the db is seeded once, to avoid enlarging the db- // Initialize outside the benchmark so the db is seed once, to avoid `unique` constraints violations + // Initialize outside the benchmark so the db is seeded once, to avoid `unique` constraint violationsAlso applies to: 90-90
195-195: Track the TODO for deserialize benchmarks.The TODO indicates that additional deserialize benchmarks requiring a typespace are not yet implemented.
Would you like me to create an issue to track implementing the remaining deserialize benchmarks that require a typespace?
SpacetimeDB/crates/bench/src/spacetime_raw.rs (2)
127-142: Consider batching operations for better performance.The current implementation deletes and inserts rows one by one. While the comment explains this mirrors module behavior, consider batching these operations for better performance in benchmarks.
Additionally, the delete-insert pattern is not atomic - if an error occurs after delete but before insert, data could be lost.
144-148: Improve error message for type mismatch.The panic message could be more informative by including the actual type found.
- if let AlgebraicValue::U64(i) = row.elements[1] { - row.elements[1] = AlgebraicValue::U64(i + 1); - } else { - panic!("column 1 is not a u64!"); - } + if let AlgebraicValue::U64(i) = row.elements[1] { + row.elements[1] = AlgebraicValue::U64(i + 1); + } else { + panic!("column 1 is not a u64, found: {:?}", row.elements[1]); + }SpacetimeDB/crates/bench/hyper_cmp.py (1)
160-160: Consider avoiding shell=True for subprocess calls.Using
shell=Truecan be a security risk. Consider restructuring the commands to avoid shell interpretation.Would you like me to help refactor these subprocess calls to avoid
shell=Truewhile maintaining the same functionality?Also applies to: 162-162, 169-169
SpacetimeDB/crates/bench/src/sqlite.rs (3)
62-66: Consider documenting supported types or returning an error.The
unimplemented!()macro will panic at runtime for unsupported algebraic types. Consider either documenting the limitation or returning a proper error.let type_ = match column.algebraic_type { AlgebraicType::U32 | AlgebraicType::U64 => "INTEGER", AlgebraicType::String => "TEXT", - _ => unimplemented!(), + _ => return Err(format!("Unsupported algebraic type: {:?}", column.algebraic_type).into()), };
184-204: Handle unsupported value types gracefully.Similar to the create_table method, the filter method will panic on unsupported AlgebraicValue types.
match value { AlgebraicValue::String(value) => { for _ in stmt.query_map((&*value,), |row| { black_box(row); Ok(()) })? {} } AlgebraicValue::U32(value) => { for _ in stmt.query_map((value,), |row| { black_box(row); Ok(()) })? {} } AlgebraicValue::U64(value) => { for _ in stmt.query_map((value,), |row| { black_box(row); Ok(()) })? {} } - _ => unimplemented!(), + _ => return Err(format!("Unsupported filter value type: {:?}", value).into()), }
255-260: Well-implemented thread-safe memoization.The query memoization system correctly uses RwLock for thread safety and implements an efficient read-lock fast path. The comment about "tuple dereferencing problems" could be clarified.
Consider documenting what specific "tuple dereferencing problems" necessitate the double hashmap structure.
SpacetimeDB/crates/bench/benches/callgrind.rs (2)
12-12: Track the resolution of disabled benchmarks.The FIXME references issue #564 for the disabled spacetime_module benchmarks. Consider adding a more specific tracking mechanism or timeline for re-enabling these benchmarks.
Would you like me to create a follow-up issue to track the re-enabling of these benchmarks once issue #564 is resolved?
86-104: Consider extracting JSON configuration to constants or builders.The inline JSON strings in attributes are error-prone and difficult to maintain. Consider using a builder pattern or extracting these to constants.
Example approach:
const INSERT_BULK_CONFIG: &str = r#"{"bench":"insert bulk", "db": "stdb_raw", "in_memory": true, "schema": "u32_u64_str", "indices": "unique_0", "preload": 128, "count": 64}"#; #[library_benchmark] #[bench::mem_unique_64(INSERT_BULK_CONFIG)] fn insert_bulk_raw_u32_u64_str(metadata: &str) { // ... }Or better yet, use a configuration builder:
fn insert_bulk_config(db: &str, in_memory: bool, indices: &str, preload: u32, count: u32) -> String { serde_json::json!({ "bench": "insert bulk", "db": db, "in_memory": in_memory, "schema": "u32_u64_str", "indices": indices, "preload": preload, "count": count }).to_string() }SpacetimeDB/.github/workflows/ci.yml (1)
56-58: Document the Windows SDK manifest issue with a tracking reference.The comment mentions that "sdk-manifests on windows-latest are messed up" but doesn't provide context on the issue or when it might be resolved.
Consider adding a TODO with an issue reference or more detailed explanation of the problem and the expected fix timeline.
SpacetimeDB/crates/bench/benches/delete_table.rs (3)
119-120: Remove#[allow(unused)]iflen()is used in benchmarks.The
#[allow(unused)]attribute on thelen()method suggests it might not be used. If it's actually used in the implementations and tests, this attribute should be removed.
251-291: Consider adding documentation for the complex binary search logic.The
find_range_to_insert_offsetfunction has complex logic with side effects during binary search. While the implementation appears correct, it would benefit from detailed documentation explaining the algorithm and the purpose of the mutableextend_endandexistsflags.
433-433: Document why DTPageAndOffsetRanges is marked as "best so far".The comment indicates this implementation performs best, but it would be helpful to include actual benchmark results or explain what makes it superior.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/Tests.cs (2)
89-91: Consider more efficient source modification approach.The current approach creates modified source by joining lines with comments. This could be more efficient using
WithChangedTextwith a simple whitespace change.- SourceText.From( - string.Join( - "\n", - tree.GetText().Lines.Select(line => $"{line} // Modified") - ) - ) + tree.GetText().WithChanges(new TextChange(new TextSpan(0, 0), " "))
113-119: Consider making the caching check more specific.The current check verifies that steps are either
CachedorUnchanged, but it might be valuable to verify that most steps are specificallyCachedrather thanUnchangedto ensure the cache is actually being used.SpacetimeDB/.github/workflows/benchmarks.yml (1)
24-353: Consider extracting common steps into reusable actions.Both
benchmarkandcallgrind_benchmarkjobs share significant logic for membership checking, comment posting, and result uploading. Consider creating composite actions to reduce duplication and improve maintainability.SpacetimeDB/crates/bench/src/spacetime_module.rs (4)
28-34: Critical: Maintain field order to prevent deadlock.The comment correctly identifies that
modulemust be dropped beforeruntime. Consider adding a compile-time assertion or test to verify this ordering constraint is maintained.Would you like me to suggest a test that verifies the drop order doesn't cause deadlock?
96-108: Track the unimplemented delete functionality.The FIXME indicates that the reducer-based approach for clearing tables is not working. While the workaround is functional for benchmarking, this should be tracked as a known issue.
Would you like me to open an issue to track implementing the
clear_table_*reducers properly?
113-129: Consider a more robust count implementation.The current implementation has limitations:
- Relies on log parsing which is fragile
- Not safe for concurrent access
- Assumes specific log message format
For benchmarking this may be acceptable, but consider implementing a dedicated count reducer that returns the value directly.
173-189: Document the reducer naming convention dependency.The filter implementation assumes specific reducer naming patterns (
filter_{table}_by_{column}). Consider adding a doc comment that explicitly documents this naming convention requirement.Add a comment like:
fn filter<T: BenchTable>( &mut self, table_id: &Self::TableId, col_id: impl Into<ColId>, value: AlgebraicValue, ) -> ResultBench<()> { let SpacetimeModule { runtime, module, .. } = self; let product_type = T::product_type(); let column_name = product_type.elements[col_id.into().idx()].name.as_ref().unwrap(); + // Assumes the module has reducers named `filter_{table}_by_{column}` let reducer_name = format!("filter_{}_by_{}", table_id.snake_case, column_name);SpacetimeDB/crates/bench/src/bin/summarize.rs (1)
469-489: Consider graceful handling of unknown metrics.Using
unimplemented!for unknown metrics will cause a panic. Consider returning the original name or logging a warning instead:fn relabel_callgrind_metric(name: &str) -> &'static str { match name { "Ir" => "instructions", // ... other mappings ... "Bi" => "indirect branches", - _ => unimplemented!("unknown callgrind metric {}", name), + _ => { + eprintln!("Warning: unknown callgrind metric '{}'", name); + return name; + } } }Note: The current implementation returns
&'static strwhich would need to change to return&strorCow<'static, str>for this approach.SpacetimeDB/crates/bench/src/schemas.rs (1)
131-134: Add a descriptive panic message for unimplemented method.The
unimplemented!()macro will panic with a generic message. Consider using a more descriptive panic message to help developers understand why this method is not implemented.- unimplemented!() + unimplemented!("u64_u64_u32 is only for special serialization benchmarks and does not support SQLite")SpacetimeDB/crates/bench/benches/generic.rs (1)
77-79: Consider removing or documenting the commented NoIndex strategy.The commented line for
NoIndexstrategy should either be removed or have a comment explaining why it's disabled.let tables: [TableData<DB::TableId>; 2] = [ prep_table(IndexStrategy::Unique0)?, - //prep_table(IndexStrategy::NoIndex)?, + // NoIndex strategy disabled: [add reason here] + // prep_table(IndexStrategy::NoIndex)?, prep_table(IndexStrategy::BTreeEachColumn)?, ];SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/Runtime.cs (1)
126-159: Consider adding formal deprecation attribute.While the comments indicate this class is deprecated, consider adding the
[Obsolete]attribute to provide compiler warnings to users.+[Obsolete("This class is deprecated due to reflection usage. Auto-generated serialization code now reads/writes the tag byte directly.")] public readonly struct Enum<T> : IReadWrite<T> where T : struct, EnumSpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Builtins.cs (1)
87-87: Follow C# naming conventions for local variables.Local variable should use camelCase naming convention.
Apply this diff:
- var NumberChars = hex.Length; + var numberChars = hex.Length;Also update the references on lines 88 and 89:
- var bytes = new byte[NumberChars / 2]; - for (var i = 0; i < NumberChars; i += 2) + var bytes = new byte[numberChars / 2]; + for (var i = 0; i < numberChars; i += 2)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
SpacetimeDB/Cargo.lockis excluded by!**/*.lockSpacetimeDB/crates/bindings-csharp/logo.pngis excluded by!**/*.pngSpacetimeDB/crates/bindings/tests/snapshots/deps__duplicate_deps.snapis excluded by!**/*.snapSpacetimeDB/crates/bindings/tests/snapshots/deps__spacetimedb_bindings_dependencies.snapis excluded by!**/*.snap
📒 Files selected for processing (107)
.gitignore(1 hunks).prettierignore(1 hunks).prettierrc.json(1 hunks)DEVELOPING.md(1 hunks)README.md(4 hunks)SpacetimeDB/.cargo/config.toml(1 hunks)SpacetimeDB/.dockerignore(1 hunks)SpacetimeDB/.env.sample(1 hunks)SpacetimeDB/.gitattributes(1 hunks)SpacetimeDB/.github/CODEOWNERS(1 hunks)SpacetimeDB/.github/pull_request_template.md(1 hunks)SpacetimeDB/.github/workflows/benchmarks.yml(1 hunks)SpacetimeDB/.github/workflows/bottest.yml(1 hunks)SpacetimeDB/.github/workflows/bottest_tracing_fix.patch(1 hunks)SpacetimeDB/.github/workflows/check-merge-labels.yml(1 hunks)SpacetimeDB/.github/workflows/check-pr-base.yml(1 hunks)SpacetimeDB/.github/workflows/ci.yml(1 hunks)SpacetimeDB/.github/workflows/discord-posts.yml(1 hunks)SpacetimeDB/.github/workflows/docker.yml(1 hunks)SpacetimeDB/.github/workflows/package.yml(1 hunks)SpacetimeDB/.github/workflows/pr-only-ci.yml(1 hunks)SpacetimeDB/.github/workflows/rust_matcher.json(1 hunks)SpacetimeDB/.github/workflows/tag-release.yml(1 hunks)SpacetimeDB/.github/workflows/upgrade-version-check.yml(1 hunks)SpacetimeDB/.gitignore(1 hunks)SpacetimeDB/.jenkins/Jenkinsfile.internal(1 hunks)SpacetimeDB/.rustfmt.toml(1 hunks)SpacetimeDB/Cargo.toml(1 hunks)SpacetimeDB/Dockerfile(1 hunks)SpacetimeDB/LICENSE.txt(1 hunks)SpacetimeDB/README.md(1 hunks)SpacetimeDB/clippy.toml(1 hunks)SpacetimeDB/crates/auth/Cargo.toml(1 hunks)SpacetimeDB/crates/auth/src/identity.rs(1 hunks)SpacetimeDB/crates/auth/src/lib.rs(1 hunks)SpacetimeDB/crates/bench/.gitignore(1 hunks)SpacetimeDB/crates/bench/Cargo.toml(1 hunks)SpacetimeDB/crates/bench/Dockerfile(1 hunks)SpacetimeDB/crates/bench/README.md(1 hunks)SpacetimeDB/crates/bench/benches/callgrind.rs(1 hunks)SpacetimeDB/crates/bench/benches/delete_table.rs(1 hunks)SpacetimeDB/crates/bench/benches/generic.rs(1 hunks)SpacetimeDB/crates/bench/benches/index.rs(1 hunks)SpacetimeDB/crates/bench/benches/special.rs(1 hunks)SpacetimeDB/crates/bench/benches/subscription.rs(1 hunks)SpacetimeDB/crates/bench/callgrind-docker.sh(1 hunks)SpacetimeDB/crates/bench/clippy.toml(1 hunks)SpacetimeDB/crates/bench/flamegraph.sh(1 hunks)SpacetimeDB/crates/bench/hyper_cmp.py(1 hunks)SpacetimeDB/crates/bench/instruments.sh(1 hunks)SpacetimeDB/crates/bench/src/bin/summarize.rs(1 hunks)SpacetimeDB/crates/bench/src/database.rs(1 hunks)SpacetimeDB/crates/bench/src/lib.rs(1 hunks)SpacetimeDB/crates/bench/src/schemas.rs(1 hunks)SpacetimeDB/crates/bench/src/spacetime_module.rs(1 hunks)SpacetimeDB/crates/bench/src/spacetime_raw.rs(1 hunks)SpacetimeDB/crates/bench/src/sqlite.rs(1 hunks)SpacetimeDB/crates/bindings-csharp/.config/dotnet-tools.json(1 hunks)SpacetimeDB/crates/bindings-csharp/.editorconfig(1 hunks)SpacetimeDB/crates/bindings-csharp/.gitignore(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/BSATN.Codegen.csproj(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/Diag.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/Type.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/Utils.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime.Tests/BSATN.Runtime.Tests.csproj(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime.Tests/Tests.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/.gitignore(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Attrs.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN.Runtime.csproj(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/AlgebraicType.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/I128.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/I256.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/Runtime.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/U128.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/U256.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Builtins.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Db.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Internal/ByteArrayComparer.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/package.json(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/Codegen.Tests.csproj(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/TestInit.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/Tests.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/Directory.Build.props(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/Lib.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/client.csproj(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomClass.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomStruct.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomTaggedEnum.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#PublicTable.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/.gitattributes(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/Lib.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/diag.csproj(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/ExtraCompilationErrors.verified.txt(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#FFI.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#InAnotherNamespace.TestDuplicateTableName.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.InAnotherNamespace.TestDuplicateReducerName.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.OnReducerWithReservedPrefix.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestDuplicateReducerKind1.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestDuplicateReducerKind2.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestDuplicateReducerName.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestReducerReturnType.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestReducerWithoutContext.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.__ReducerWithReservedPrefix.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestAutoIncNotInteger.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestDuplicateTableName.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestIndexIssues.verified.cs(1 hunks)SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestScheduleIssues.DummyScheduledReducer.verified.cs(1 hunks)
⛔ Files not processed due to max files limit (25)
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestScheduleIssues.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestTableTaggedEnum.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestUniqueNotEquatable.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module.verified.txt
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Type#TestTaggedEnumField.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Type#TestTaggedEnumInlineTuple.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Type#TestTypeParams_T_.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Type#TestUnsupportedType.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Type.verified.txt
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/Lib.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/server.csproj
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#BTreeMultiColumn.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#BTreeViews.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#FFI.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#MultiTableRow.InsertMultiData.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#MultiTableRow.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#PrivateTable.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#PublicTable.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#Reducers.InsertData.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#Reducers.ScheduleImmediate.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#RegressionMultipleUniqueIndexesHadSameName.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#Test.NestingNamespaces.AndClasses.InsertData2.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#Timers.Init.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#Timers.SendMessageTimer.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#Timers.SendScheduledMessage.verified.cs
✅ Files skipped from review due to trivial changes (50)
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/.gitattributes
- SpacetimeDB/.cargo/config.toml
- SpacetimeDB/crates/bench/.gitignore
- SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/.gitignore
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/diag.csproj
- SpacetimeDB/crates/bench/clippy.toml
- SpacetimeDB/.env.sample
- SpacetimeDB/.github/workflows/bottest_tracing_fix.patch
- SpacetimeDB/.rustfmt.toml
- SpacetimeDB/.gitattributes
- SpacetimeDB/.github/workflows/check-pr-base.yml
- SpacetimeDB/.github/CODEOWNERS
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestReducerReturnType.verified.cs
- SpacetimeDB/crates/bindings-csharp/.gitignore
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/client.csproj
- SpacetimeDB/.dockerignore
- SpacetimeDB/crates/auth/src/lib.rs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestDuplicateReducerName.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.OnReducerWithReservedPrefix.verified.cs
- .gitignore
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/Directory.Build.props
- SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Attrs.cs
- SpacetimeDB/crates/bench/instruments.sh
- SpacetimeDB/.github/workflows/upgrade-version-check.yml
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.InAnotherNamespace.TestDuplicateReducerName.verified.cs
- SpacetimeDB/.github/workflows/rust_matcher.json
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestReducerWithoutContext.verified.cs
- SpacetimeDB/crates/bindings-csharp/.config/dotnet-tools.json
- SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/package.json
- .prettierrc.json
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestScheduleIssues.DummyScheduledReducer.verified.cs
- SpacetimeDB/crates/bindings-csharp/.editorconfig
- .prettierignore
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.__ReducerWithReservedPrefix.verified.cs
- SpacetimeDB/crates/bench/Dockerfile
- SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN.Runtime.csproj
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestDuplicateReducerKind1.verified.cs
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#Reducers.TestDuplicateReducerKind2.verified.cs
- SpacetimeDB/crates/bench/callgrind-docker.sh
- SpacetimeDB/clippy.toml
- SpacetimeDB/.github/workflows/check-merge-labels.yml
- SpacetimeDB/.github/pull_request_template.md
- SpacetimeDB/crates/bindings-csharp/BSATN.Runtime.Tests/BSATN.Runtime.Tests.csproj
- SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/BSATN.Codegen.csproj
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/Codegen.Tests.csproj
- SpacetimeDB/.gitignore
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestDuplicateTableName.verified.cs
- SpacetimeDB/crates/auth/Cargo.toml
- SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestAutoIncNotInteger.verified.cs
- SpacetimeDB/crates/bench/Cargo.toml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
SpacetimeDB/.github/workflows/tag-release.yml
[error] 18-18: trailing spaces
(trailing-spaces)
[error] 19-19: trailing spaces
(trailing-spaces)
🪛 Pylint (3.3.7)
SpacetimeDB/crates/bench/hyper_cmp.py
[error] 10-10: Undefined variable 'namedtuple'
(E0602)
[refactor] 58-58: Too few public methods (0/2)
(R0903)
[refactor] 70-70: Too few public methods (0/2)
(R0903)
[refactor] 77-77: Consider using 'with' for resource-allocating operations
(R1732)
[refactor] 83-86: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 133-133: Consider using '{"SpacetimeDB": stat.spacetime['mean'], "Sqlite": stat.sqlite['mean'], ... }' instead of a call to 'dict'.
(R1735)
[refactor] 147-147: Consider using '{"Old": old.spacetime['mean'], "New": new.spacetime['mean']}' instead of a call to 'dict'.
(R1735)
🪛 Ruff (0.11.9)
SpacetimeDB/crates/bench/hyper_cmp.py
2-2: os imported but unused
Remove unused import: os
(F401)
10-10: Undefined name namedtuple
(F821)
77-77: Use a context manager for opening files
(SIM115)
🪛 LanguageTool
DEVELOPING.md
[uncategorized] ~1-~1: Possible missing comma found.
Context: ...ls. Here is the updated developer setup guide incorporating your feedback. --- # Ax...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~21-~21: For conciseness, consider replacing this expression with an adverb.
Context: ...t required for developing the dashboard at this moment, you can prepare your system by followi...
(AT_THE_MOMENT)
[uncategorized] ~55-~55: Possible missing comma found.
Context: ...tall all dependencies for the SvelteKit frontend located in /dashboard. 4. **Log in ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~81-~81: Possible missing comma found.
Context: ...m the /server directory. This step is crucial as it also **generates the TypeScript c...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~139-~139: Loose punctuation mark.
Context: ...6. Project Structure - /dashboard: The SvelteKit frontend application. - *...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~140-~140: Loose punctuation mark.
Context: ...it frontend application. - /server: The Rust modules for the SpaceTimeDB ba...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~141-~141: Loose punctuation mark.
Context: ...he SpaceTimeDB backend. - /scripts: Utility scripts for development tasks. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~142-~142: Loose punctuation mark.
Context: ... development tasks. - /SpacetimeDB: A git subtree of the official SpaceTime...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~143-~143: Loose punctuation mark.
Context: ...ed to build local binaries. - /bin: Contains the compiled spacetimedb-cli...
(UNLIKELY_OPENING_PUNCTUATION)
README.md
[uncategorized] ~52-~52: Loose punctuation mark.
Context: ...ational code for Axonotes: - /app: - The Axonotes desktop application...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~59-~59: Loose punctuation mark.
Context: ...ation with the backend. - /server: - The backend logic and data module...
(UNLIKELY_OPENING_PUNCTUATION)
SpacetimeDB/LICENSE.txt
[uncategorized] ~34-~34: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...s document, or the “License”) is not an Open Source license. However, the Licensed Work wil...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~36-~36: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...l eventually be made available under an Open Source License, as stated in this License. Li...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~65-~65: Possible missing comma found.
Context: ...rately for each version of the Licensed Work and the Change Date may vary for each v...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~106-~106: Conjunctions like ‘or’ should not follow semicolons. Use a comma instead.
Context: ... License, as the Additional Use Grant; or (b) insert the text “None”. 3. To spec...
(CONJUNCTION_AFTER_SEMICOLON)
[grammar] ~128-~128: Possible agreement error. The noun ‘section’ seems to be countable.
Context: ...onal permission under GNU GPL version 3 section 7 If you modify this Program, or any c...
(CD_NN)
[grammar] ~135-~135: Possible agreement error. The noun ‘section’ seems to be countable.
Context: ...nal permission under GNU AGPL version 3 section 13 If you modify this Program, or any ...
(CD_NN)
[style] ~163-~163: Consider using only “Public” to avoid wordiness.
Context: ... and change the works. By contrast, our General Public Licenses are intended to guarantee your...
(GENERAL_XX)
[style] ~168-~168: Consider using only “Public” to avoid wordiness.
Context: ...e referring to freedom, not price. Our General Public Licenses are designed to make sure that...
(GENERAL_XX)
[style] ~175-~175: Consider using only “Public” to avoid wordiness.
Context: ... these things. Developers that use our General Public Licenses protect your rights with two ...
(GENERAL_XX)
[uncategorized] ~186-~186: Possible missing comma found.
Context: ... and letting the public access it on a server without ever releasing its source code...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~217-~217: Possible missing comma found.
Context: ...led a "modified version" of the earlier work or a work "based on" the earlier work....
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~288-~288: Possible missing comma found.
Context: ...d works that you do not convey, without conditions so long as your license otherwise remai...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~310-~310: ‘To effect’ means ‘to cause’. Did you mean “affected”?
Context: ...es to the extent such circumvention is effected by exercising rights under this License...
(EFFECT_AFFECT)
[style] ~311-~311: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...by exercising rights under this License with respect to the covered work, and you disclaim any...
(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
[style] ~389-~389: Consider using only “public” to avoid wordiness.
Context: ...ce of the work are being offered to the general public at no charge under subsection 6d. A sep...
(GENERAL_XX)
[style] ~501-~501: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...the violation by some reasonable means prior to 60 days after the cessation. Moreover,...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~507-~507: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ght holder, and you cure the violation prior to 30 days after your receipt of the notic...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~515-~515: Consider a more concise word here.
Context: ...are not required to accept this License in order to receive or run a copy of the Program. A...
(IN_ORDER_TO_PREMIUM)
[style] ~516-~516: To make your writing clearer, consider a shorter, more direct phrase.
Context: ...tion of a covered work occurring solely as a consequence of using peer-to-peer transmission to rece...
(AS_A_CONSEQUENCE_OF)
[style] ~557-~557: To make your writing clearer, consider a shorter, more direct phrase.
Context: ...de claims that would be infringed only as a consequence of further modification of the contributo...
(AS_A_CONSEQUENCE_OF)
[grammar] ~569-~569: ‘an’ may be redundant when used with the uncountable noun ‘permission’.
Context: ...ated, not to enforce a patent (such as an express permission to practice a patent or covenant not to...
(A_UNCOUNTABLE_NOUN)
[grammar] ~581-~581: The verb ‘rely’ requires the preposition ‘on’ (or ‘upon’).
Context: ...se to downstream recipients. "Knowingly relying" means you have actual knowledge that, ...
(RELY_ON)
[misspelling] ~582-~582: Did you mean “you're” (short for ‘you are’)?
Context: ...ledge that, but for the patent license, your conveying the covered work in a countr...
(YOUR)
[uncategorized] ~600-~600: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...conveying the work, and under which the third party grants, to any of the parties who woul...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~606-~606: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...t, or that patent license was granted, prior to 28 March 2007. Nothing in this License...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~615-~615: ‘So as to’ expresses purpose and is used in formal texts. Consider using “to”.
Context: ...e. If you cannot convey a covered work so as to satisfy simultaneously your obligations...
(SO_AS_TO)
[uncategorized] ~666-~666: Possible missing comma found.
Context: ...E LAW. EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT HOLDERS AND/OR OTHER PAR...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~701-~701: Conjunctions like ‘and’ should not follow semicolons. Consider using a comma, or removing the conjunction.
Context: ...ectively state the exclusion of warranty; and each file should have at least the "copyrigh...
(CONJUNCTION_AFTER_SEMICOLON)
SpacetimeDB/README.md
[uncategorized] ~72-~72: Possible missing comma found.
Context: ...d authorization logic right inside your module just as you would in a normal server. ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~85-~85: Consider removing “of” to be more concise
Context: ...hich means that everything in the game, all of the chat messages, items, resources, terrai...
(ALL_OF_THE)
[style] ~85-~85: Consider removing “of” to be more concise
Context: ...tabase before being synchronized out to all of the clients in real-time. SpacetimeDB is o...
(ALL_OF_THE)
[uncategorized] ~87-~87: Possible missing comma found.
Context: ...optimized for maximum speed and minimum latency rather than batch processing or OLAP wo...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~89-~89: Consider using “all application” or “all of the application”.
Context: ...peed and latency is achieved by holding all of application state in memory, while persisting the d...
(ALL_MOST_SOME_OF_NOUN)
[grammar] ~99-~99: The word ‘install’ is not a noun.
Context: ...ng on macOS is as simple as running our install script. After that you can use the spac...
(A_INSTALL)
[typographical] ~99-~99: Consider adding a comma here.
Context: ...s simple as running our install script. After that you can use the spacetime command to ma...
(FOR_THAT_COMMA)
[grammar] ~107-~107: The word ‘install’ is not a noun.
Context: ...ng on Linux is as simple as running our install script. After that you can use the spac...
(A_INSTALL)
[typographical] ~107-~107: Consider adding a comma here.
Context: ...s simple as running our install script. After that you can use the spacetime command to ma...
(FOR_THAT_COMMA)
[uncategorized] ~127-~127: Possible missing comma found.
Context: ...acOS + Linux is pretty straightforward. First we are going to build all of the binari...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~127-~127: Consider removing “of” to be more concise
Context: ...ghtforward. First we are going to build all of the binaries that we need: ```bash # Insta...
(ALL_OF_THE)
[typographical] ~149-~149: It appears that a comma is missing.
Context: ...pacetime/bin/$STDB_VERSION ``` At this stage you'll need to add ~/.local/bin to your...
(DURING_THAT_TIME_COMMA)
[grammar] ~171-~171: “Windows” (operating system by Microsoft) is a proper noun and needs to be capitalized.
Context: ...--version`. ##### Windows Building on windows is a bit more complicated. You'll need ...
(A_WINDOWS)
[uncategorized] ~171-~171: Possible missing comma found.
Context: ...ou may also need access to an openssl binary which actually comes pre-installed with...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~173-~173: Possible missing comma found.
Context: ...rs/) for Windows. In a Git for Windows shell you should have something that looks li...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~183-~183: Consider adding a comma.
Context: ...rgo/bin/cargo If that looks correct then you're ready to proceed! powershell...
(IF_THEN_COMMA)
[uncategorized] ~210-~210: Possible missing comma found.
Context: ...PROFILE%\AppData\Local\SpacetimeDB ``` Then finally, open a new shell and use the i...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~221-~221: A comma might be missing here.
Context: ...me --version`. If you're using Git for Windows you can follow these instructions inste...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[typographical] ~259-~259: Consider adding a comma here.
Context: ...velopment guides, and reference material please see our [documentation](https://spaceti...
(PLEASE_COMMA)
[uncategorized] ~265-~265: A comma might be missing here.
Context: ...age](https://spacetimedb.com/docs). In summary there are only 4 steps to getting start...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~272-~272: Possible missing comma found.
Context: ...ee a summary of the supported languages below with a link to the getting started guid...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~291-~291: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...der the BSL 1.1 license. This is not an open source or free software license, however, it c...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~291-~291: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ... an open source or free software license, however, it converts to the AGPL v3.0 license wi...
(HOWEVER_SENTENCE)
SpacetimeDB/crates/bench/README.md
[style] ~95-~95: Consider removing “of” to be more concise
Context: ... 'stdb_raw/.*/insert_bulk' ``` Will run all of the insert_bulk benchmarks against the ra...
(ALL_OF_THE)
[grammar] ~101-~101: The word ‘disc’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ... against all databases, without writing to disc. ## Workload Benches The workload ben...
(VB_TO_NN_DT)
[grammar] ~151-~151: This phrase is duplicated. You should probably use “Generic benchmarks” only once.
Context: ...Put them in benches/special.rs. #### Generic benchmarks Generic benchmarks live in benches/generic.rs. These ben...
(PHRASE_REPETITION)
[uncategorized] ~154-~154: Loose punctuation mark.
Context: ...s: - BenchDatabase, which is implemented by different datab...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~155-~155: Loose punctuation mark.
Context: ...ckends. - BenchTable, which is implemented by different bench...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~162-~162: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...cetimeModule](src/spacetime_module.rs) will require you to add a reducer to the [b...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/ExtraCompilationErrors.verified.txt
[duplication] ~142-~142: Possible typo: you repeated a word.
Context: ...ption UnsupportedSystemType; public UnresolvedType UnresolvedType; ^^^^^^^^^^^^^^ public L...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
README.md
54-54: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
60-60: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
61-61: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
79-79: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
80-80: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
81-81: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
83-83: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
84-84: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
87-87: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
88-88: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
89-89: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
137-137: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
138-138: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
139-139: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
140-140: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
142-142: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
144-144: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
145-145: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
SpacetimeDB/crates/bench/README.md
33-33: Bare URL used
null
(MD034, no-bare-urls)
49-49: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
148-148: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
160-160: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
161-161: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
162-162: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (91)
SpacetimeDB/.jenkins/Jenkinsfile.internal (3)
16-18: Pipeline agent declaration is fine
Usingagent anycovers all available executors. If you require dedicated or specialized nodes, specify a label here.
20-26: Good practice: pending status before SCM checkout
Setting the GitHub status toPENDINGprior tocheckout scmclearly signals build start.
27-44: Prevent downstream stage from always targeting master
Hard-codingbranchName = 'master'means internal tests will never run against feature branches. If the downstream job should mirror the PR branch, useenv.BRANCH_NAMEor a parameter.-def branchName = 'master' // branch to trigger +def branchName = env.BRANCH_NAME // trigger internal tests on the same branchPlease confirm whether internal tests must always run on master or follow the PR branch.
SpacetimeDB/crates/auth/src/identity.rs (4)
1-3: LGTM: Clean re-exports for JWT functionality.The re-exports provide a clean API surface for JWT error handling and key management.
29-49: Excellent flexible audience deserialization.The custom deserializer elegantly handles both single string and array formats for the JWT audience claim, which is essential for JWT standard compliance.
91-101: Robust identity validation logic.The identity computation and validation logic is well-implemented, ensuring that provided identity tokens match the computed identity from issuer and subject claims.
120-184: Comprehensive test coverage for audience deserialization.The test suite thoroughly covers the flexible audience deserialization scenarios including single string, multiple strings, and missing field cases.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Db.cs (1)
4-4: Consider if the 'new()' constraint is necessary.The generic constraint requires
DbViewto have a parameterless constructor. Evaluate if this restriction is needed or if it could limit flexibility for dependency injection scenarios.The
new()constraint might be too restrictive for advanced scenarios. Consider if the abstraction should support:
- Dependency injection patterns
- Factory methods for
DbViewconstruction- Immutable
DbViewimplementations that don't require parameterless constructorsSpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/ExtraCompilationErrors.verified.txt (1)
1-186: Auto-generated diagnostic snapshot
This file is a verified snapshot used by the C# codegen tests; no manual edits are required.🧰 Tools
🪛 LanguageTool
[duplication] ~142-~142: Possible typo: you repeated a word.
Context: ...ption UnsupportedSystemType; public UnresolvedType UnresolvedType; ^^^^^^^^^^^^^^ public L...(ENGLISH_WORD_REPEAT_RULE)
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Internal/ByteArrayComparer.cs (1)
10-23: LGTM - Well-implemented equality check.The equality implementation properly handles null references, reference equality optimization, and length mismatches before performing element-wise comparison.
SpacetimeDB/Dockerfile (3)
1-8: LGTM - Excellent multi-stage build setup.The builder stage properly uses the Rust base image and builds the necessary binaries with appropriate flags (--release, --locked).
19-30: Well-implemented multi-architecture .NET installation.The architecture detection and conditional .NET installation properly handles both amd64 and arm64 architectures with appropriate error handling for unsupported architectures.
44-46: Good security practice with non-root user.Creating and switching to a non-root user follows Docker security best practices.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/TestInit.cs (3)
12-56: Excellent diagnostic converter implementation.The custom
DiagConverterprovides outstanding error reporting with source code snippets and visual indicators (squiggly underlines) for error locations. This will significantly improve the debugging experience for generated code issues.
61-65: Smart CI vs local environment handling.The conditional diff output handling properly distinguishes between CI and local development environments, providing optimal UX for both scenarios.
70-93: Well-designed code formatting integration.The CSharpier integration ensures stable snapshots by normalizing code formatting and appends compilation errors as comments while preserving meaningful line numbers.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/U256.cs (1)
69-80: LGTM: Comparison operators are correctly implemented.The comparison operators correctly handle 256-bit comparisons by first comparing the upper parts and then the lower parts when the upper parts are equal.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/U128.cs (2)
71-82: LGTM: Comparison operators are correctly implemented.The comparison operators correctly handle 128-bit comparisons by first comparing the upper 64-bits and then the lower 64-bits when the upper parts are equal.
1-3: LGTM: Proper licensing attribution.Good practice to include the .NET Foundation license header for code that appears to be derived from or inspired by .NET Foundation implementations.
SpacetimeDB/.github/workflows/docker.yml (2)
32-54: LGTM: Proper Docker build caching strategy.The caching configuration correctly uses local cache with proper key management to avoid cache growth issues.
74-77: ARM64 specific setup steps are appropriate.The ARM64 job correctly includes additional setup steps for
jqinstallation and git reference pruning, which are necessary for ARM runners.SpacetimeDB/.github/workflows/package.yml (2)
80-88: LGTM: Secure artifact upload configuration.The upload configuration correctly uses secrets for credentials and variables for bucket configuration, following security best practices.
12-25: LGTM: Comprehensive platform coverage.The matrix strategy covers all major platforms and architectures that SpacetimeDB users are likely to need.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/I256.cs (2)
103-108: LGTM - Efficient BigInteger conversion.The
AsBigInt()method efficiently converts the 256-bit value toBigIntegerusing memory marshalling, which is the optimal approach for string representation.
101-101:✅ Verification successful
Verify the sign detection logic for edge cases.
The
IsNegativemethod castsvalue._upper.Uppertolongand checks if it's less than 0. Ensure this correctly handles the sign bit for all possible U128 values.
🏁 Script executed:
#!/bin/bash # Verify that U128.Upper property exists and represents the most significant bits ast-grep --pattern 'struct U128 { $$$ Upper $$$ $$$ }'Length of output: 62
🏁 Script executed:
#!/bin/bash # Search for the U128 struct definition in C# sources rg "\\bstruct U128\\b" -A5 --glob "*.cs" # Search for any declaration or reference of the `Upper` member in C# files rg "\\bUpper\\b" -n --glob "*.cs"Length of output: 2194
🏁 Script executed:
#!/bin/bash # Show U128 struct definition sed -n '1,200p' SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/U128.cs # Show I256 struct definition and IsNegative method sed -n '1,200p' SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/I256.csLength of output: 5896
Sign detection logic is correct
The cast
(long)value._upper.Upper < 0precisely checks bit 255 of the 256-bit two’s-complement value, returning true exactly when the sign bit is set. No changes required.SpacetimeDB/Cargo.toml (1)
126-129: Good practice: Preventing getrandom incompatibility.The comment and configuration for
ahashto preventgetrandomversion conflicts shows excellent attention to module dependency management.SpacetimeDB/LICENSE.txt (2)
1-31: LGTM - Comprehensive licensing structure.The Business Source License parameters are clearly defined with appropriate restrictions for SpacetimeDB. The Additional Use Grant allows single-instance production use while preventing database service offerings, which aligns with typical BSL usage patterns.
24-24: Verify the Change Date is appropriate.The Change Date is set to 2030-06-04, which provides approximately 5 years of protection under BSL before converting to AGPL v3.0. Ensure this timeline aligns with business objectives.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomTaggedEnum.verified.cs (3)
24-35: LGTM - Robust tagged union deserialization.The deserialization logic correctly reads the tag byte and dispatches to appropriate variant constructors, with proper error handling for invalid tags.
76-76: Good null safety in hash code calculation.The hash code implementation correctly handles potential null strings in
StringVariant, returning 0 for null values which is appropriate.
55-61: Well-structured algebraic type registration.The type registration correctly creates a Sum type with named variants, properly integrating with the BSATN type system for schema generation.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/I128.cs (4)
9-21: Well-designed struct with proper endianness handling.The sequential layout and conditional field ordering ensure correct memory representation across different architectures.
70-96: Correct signed comparison implementation.The operators properly handle signed comparisons by checking signs first, then comparing magnitudes when signs match.
101-109: Efficient BigInteger conversion with proper endianness handling.The use of
stackallocandMemoryMarshalprovides an efficient conversion path while correctly handling system endianness.
111-118: Correct sign extension for int to I128 conversion.The implementation properly sign-extends the 32-bit integer to 128 bits using arithmetic right shift.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/AlgebraicType.cs (1)
22-52: Comprehensive algebraic type system implementation.The tagged enum design supports all necessary type variants. The
MakeOptionmethod correctly implements the standard Option pattern with "some" and "none" variants.SpacetimeDB/crates/bench/src/database.rs (1)
1-53: File appears unrelated to authentication flow mentioned in PR title.This benchmarking trait is well-designed and comprehensive, but it doesn't seem related to "Complete Authflow with WorkOS and SpaceTimeDB" as mentioned in the PR objectives.
The trait itself provides a good abstraction for database benchmarking with appropriate methods for performance testing.
Likely an incorrect or invalid review comment.
SpacetimeDB/crates/bench/README.md (1)
1-8: Mismatch between PR objectives and file contentThe PR objectives indicate this is about "Complete Authflow with WorkOS and SpaceTimeDB", but this file introduces a benchmarking suite for SpacetimeDB. This seems unrelated to authentication flow implementation.
Likely an incorrect or invalid review comment.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#TestIndexIssues.verified.cs (1)
1-109: Auto-generated test fixture looks correctThe generated code follows proper patterns for serialization, equality, and null handling. The implementation is consistent with C# best practices.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomClass.verified.cs (1)
1-111: Auto-generated code follows consistent patternsThe generated CustomClass struct properly handles nullable string fields and implements equality/serialization correctly.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#PublicTable.verified.cs (1)
1-351: Comprehensive auto-generated struct implementationThe PublicTable struct correctly handles a complex set of fields including nullable types, collections, and custom types. The equality and hashing implementations properly handle all edge cases.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#InAnotherNamespace.TestDuplicateTableName.verified.cs (1)
54-63: Verify intentional equality implementation for test fixture.All instances are equal with the same hash code (0). While this may be intentional for a diagnostic test fixture, it violates the general contract for equality and hashing.
If this is not intentional for testing edge cases, consider implementing proper equality based on type identity or making instances not equal to each other.
SpacetimeDB/crates/bench/benches/subscription.rs (1)
1-243: File appears unrelated to PR objectives.The PR is titled "Complete Authflow with WorkOS and SpaceTimeDB" but this file implements subscription query benchmarks with no authentication-related code.
Please verify if this file was intended to be part of this PR or if there's a mismatch in the PR contents.
Likely an incorrect or invalid review comment.
SpacetimeDB/.github/workflows/bottest.yml (1)
102-102:⚠️ Potential issueFix syntax error in secret reference.
Missing space between
AWS_SECRET_ACCESS_KEYand}}.- aws_secret_access_key: ${{ secrets.AWS_SECRET_ACCESS_KEY}} + aws_secret_access_key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}Likely an incorrect or invalid review comment.
SpacetimeDB/crates/bench/src/lib.rs (1)
124-131: Good documentation of concurrency constraints.The comment clearly explains why serial execution is necessary for module tests, helping future maintainers understand the constraint.
SpacetimeDB/crates/bench/benches/special.rs (2)
16-25: Appropriate allocator selection for benchmarking.Good choice of allocators - mimalloc for MSVC targets and jemalloc for others. This ensures optimal memory performance across different platforms during benchmarking.
144-146:✅ Verification successful
Verify PagePool resource management.
The
PagePool::new_for_test()is created but it's unclear if it requires explicit cleanup. Please verify that resources are properly released after benchmarking.
🏁 Script executed:
#!/bin/bash # Search for PagePool implementation to understand cleanup requirements ast-grep --pattern 'impl Drop for PagePool { $$$ }' # Check if new_for_test has special cleanup semantics rg -A 10 "new_for_test.*PagePool"Length of output: 89
Here’s a deeper search to locate the
PagePooldefinition and any cleanup logic:
🏁 Script executed:
#!/bin/bash set -eux # Find where PagePool is defined rg --no-heading --line-number "struct PagePool" -n . # Search for Drop implementation for PagePool rg --no-heading --line-number "impl Drop for PagePool" -n . # Find new_for_test definition and usage for PagePool rg --no-heading --line-number "new_for_test" -n .Length of output: 267
No explicit cleanup required for PagePool
Verification shows that
PagePool::new_for_test()inSpacetimeDB/crates/table/src/page_pool.rshas noDropimpl nor does it spawn background threads. Rust’s ownership model and RAII will release all resources when the pool goes out of scope, so no manual cleanup is needed after benchmarking.SpacetimeDB/crates/bench/src/spacetime_raw.rs (1)
159-164: Correct use of black_box for benchmarking.Good use of
black_boxto prevent the compiler from optimizing away the iteration. This ensures accurate benchmark measurements.Also applies to: 167-179
SpacetimeDB/crates/bench/hyper_cmp.py (1)
30-56: Well-implemented Unicode bar chart visualization.Excellent implementation with clear comments explaining the Unicode block element logic. The handling of fractional widths and edge cases is thorough.
SpacetimeDB/crates/bench/src/sqlite.rs (2)
18-23: Good design keeping the temp directory alive.The
_temp_dirfield ensures the temporary directory persists for the lifetime of the SQLite instance, preventing premature cleanup.
40-43: Appropriate SQLite configuration for benchmarking.Setting
journal_mode = WALandsynchronous = offcorrectly aligns with SpacetimeDB'swal_fsync=falsefor fair performance comparison.SpacetimeDB/crates/bench/benches/callgrind.rs (1)
594-598: Critical configuration properly documented.Good job highlighting the importance of the custom entry point configuration. This prevents subtle breakage of the benchmarking infrastructure.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/Lib.cs (2)
48-76: Comprehensive test fixture structure.The
PublicTablestruct provides excellent coverage of various data types for testing the SpacetimeDB C# bindings, including primitives, custom types, collections, and nullable types.
39-42:✅ Verification successful
Verify if IsExternalInit workaround is still needed.
This workaround enables C# 9 init-only properties on older frameworks. Verify if it's still necessary based on your target framework.
🌐 Web query:
What .NET target frameworks require the IsExternalInit workaround for C# 9 init properties?💡 Result:
.NET Target Frameworks Requiring the IsExternalInit Workaround for C# 9 Init Properties
C# 9 introduced init-only properties, enabling property setters that can only be set during initialization. However, this feature relies on the presence of the type
System.Runtime.CompilerServices.IsExternalInit, which is not present in all versions of .NET. Here’s when you need the workaround and why:Which Target Frameworks Require the Workaround?
The IsExternalInit workaround is needed when you use C# 9’s
initproperties orrecordtypes in projects that target frameworks older than .NET 5.0. Specifically, you need this workaround for:
- .NET Framework 4.x and below[4][6][7]
- .NET Core 2.x and 3.x[4][6][7]
- .NET Standard 2.0 and 2.1[2][4][5][6][7]
- .NET Standard 1.x (if using a suitable C# language version setting)[8]
.NET 5.0 and later include
IsExternalInitin the base libraries, so frameworks net5.0 and newer do not require the workaround[1][7].Why Is the Workaround Required?
- The C# 9 compiler emits a requirement for the type
System.Runtime.CompilerServices.IsExternalInitwhen usinginitproperties[9].- Only .NET 5.0+ defines this type.
- When targeting older frameworks, the compiler will raise an error unless it finds this type, so you must define it yourself (commonly as an
internal static classor similar in your project)[1][4][6][7].Typical Workaround Implementation
Add the following code to your project (e.g., in a file like
IsExternalInit.cs):namespace System.Runtime.CompilerServices { internal static class IsExternalInit { } }Or, you can use NuGet packages like IsExternalInit to do this automatically[1][2].
Summary Table
Target Framework IsExternalInit Included? Workaround Needed? .NET Framework 4.x and below No Yes .NET Core 2.x No Yes .NET Core 3.x No Yes .NET Standard 2.0, 2.1 No Yes .NET 5.0 and above Yes No Additional Notes
- You must also set the C# language version to 9.0 (or later) in your project file to use these features on older target frameworks[4][8].
- If you multi-target (e.g., both netstandard2.0 and net8.0), you may need to conditionally compile the workaround to avoid type duplication in new targets[5].
"The IsExternalInit type is only included in the net5.0 (and subsequent) target frameworks, so one cannot use it right away in a NetStandard 2.0 (or 2.1, for that matter) library."[7]
In summary, any target framework older than .NET 5.0 requires the IsExternalInit workaround to use init-only properties and records from C# 9[1][4][6][7][8].
Citations:
- 1: https://github.com/manuelroemer/IsExternalInit
- 2: https://www.nuget.org/packages/IsExternalInit
- 3: https://learn.microsoft.com/en-us/dotnet/standard/frameworks
- 4: https://blog.ndepend.com/using-c9-record-and-init-property-in-your-net-framework-4-x-net-standard-and-net-core-projects/
- 5: https://sergeyteplyakov.github.io/Blog/c%23/2024/03/06/CSharp_Language_Features_vs_Target_Frameworks.html
- 6: https://btburnett.com/csharp/2020/12/11/csharp-9-records-and-init-only-setters-without-dotnet5.html
- 7: https://nicolaiarocci.substack.com/p/making-the-latest-c-language-features
- 8: https://montemagno.com/enabling-c-9-in-xamarin-net-standard-projects/
- 9: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-9.0/init
- 10: https://devblogs.microsoft.com/dotnet/c-9-0-on-the-record/
🏁 Script executed:
#!/bin/bash set -e # List all .csproj files under bindings-csharp echo "=== .csproj files ===" fd -e csproj SpacetimeDB/crates/bindings-csharp # Display the TargetFramework(s) for the Codegen.Tests project echo -e "\n=== Codegen.Tests csproj content ===" cat SpacetimeDB/crates/bindings-csharp/Codegen.Tests/*.csproj | grep -E "<TargetFrameworks?>"Length of output: 553
🏁 Script executed:
#!/bin/bash set -e # Find all .csproj files under bindings-csharp projects=$(fd -e csproj | grep bindings-csharp) if [ -z "$projects" ]; then echo "No .csproj files found under bindings-csharp." exit 1 fi # Display the TargetFramework(s) for each project for proj in $projects; do echo "=== $proj ===" grep -E "<TargetFrameworks?>.*</TargetFrameworks?>" "$proj" || echo " (no TargetFramework found)" doneLength of output: 4665
No changes needed: IsExternalInit workaround is required
The
Codegen.Tests/fixtures/clientproject targets netstandard2.1, which is older than .NET 5.0 and does not includeSystem.Runtime.CompilerServices.IsExternalInit. Keeping this internal definition is necessary to enable C# 9 init-only properties in that fixture.• SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/client/client.csproj → netstandard2.1 (workaround needed)
SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/Diag.cs (3)
22-112: Well-designed type-safe diagnostic framework.The use of LINQ expressions to create type-safe diagnostics is elegant and prevents format string mismatches. The multiple constructor overloads provide good flexibility for different Roslyn syntax types.
225-250: Excellent defensive programming with exception handling.The catch-all exception handler provides a safety net to prevent the code generator from crashing, instead limiting the damage to skipping individual syntax nodes. This is a best practice for Roslyn generators.
158-165:⚠️ Potential issueFix off-by-one error in array access.
The condition checks if
Members.Count > 256, but the array accessMembers[256]will throw anIndexOutOfRangeExceptionwhen the count is exactly 256 (valid indices are 0-255).Apply this fix:
- group, - "[SpacetimeDB.Type] enums are limited to 256 variants", - @enum => - $"{@enum.Identifier} has {@enum.Members.Count} variants which is more than the allowed 256 variants for SpacetimeDB enums.", - @enum => @enum.Members[256] + group, + "[SpacetimeDB.Type] enums are limited to 256 variants", + @enum => + $"{@enum.Identifier} has {@enum.Members.Count} variants which is more than the allowed 256 variants for SpacetimeDB enums.", + @enum => @enum.Members[255]Likely an incorrect or invalid review comment.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime.Tests/Tests.cs (2)
10-134: Excellent test coverage for ConnectionId and Identity types.The tests thoroughly validate roundtrip conversions, edge cases with property-based testing, and proper error handling for invalid inputs. The use of CsCheck for property-based testing is a best practice.
695-777: Comprehensive ToString() testing with good edge case coverage.The tests validate string representations for various scenarios including null values, empty collections, and truncation for large collections. This will greatly aid in debugging.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#FFI.verified.cs (1)
1-1055: LGTM! Generated code follows consistent patterns.The auto-generated FFI bindings are well-structured with proper implementations for table views, indexes, and reducer registration. The conditional compilation for WASM AOT mode is correctly handled.
SpacetimeDB/crates/bench/src/spacetime_module.rs (1)
36-42: Good overhead analysis for block_on usage.The 70ns overhead from
block_onis indeed negligible compared to the 20μs reducer invocation overhead, making this a reasonable design choice for the benchmarking interface.SpacetimeDB/crates/bench/src/bin/summarize.rs (1)
72-86: Good defensive check against repacking.The assertion preventing repacking of
.jsonfiles is a good safeguard. The implementation correctly gathers benchmarks and serializes them with pretty printing.SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/Utils.cs (6)
11-29: LGTM! Well-implemented equatable array wrapper.The
EquatableArray<T>implementation correctly provides deep equality semantics for immutable arrays, which is essential for incremental source generators. The use ofStructuralEqualityComparerfor hash code generation is appropriate.
43-71: LGTM! Robust file name sanitization.The conservative approach to sanitizing file names is appropriate given Roslyn's undocumented character restrictions. Using
SyntaxFacts.IsIdentifierPartCharacterensures generated file names will be valid.
158-163: Clarify the "Stdb" suffix naming convention.The naming convention for 128-bit integer types is inconsistent:
System.Int128maps toI128SpacetimeDB.I128maps toI128StdbThis could be confusing for users. Consider documenting why the "Stdb" suffix is used or adopting a more consistent naming scheme.
202-217: LGTM! Proper typed array handling.The
ResolveConstantmethod correctly preserves array element types usingArray.CreateInstance, avoiding the common pitfall of creatingobject[]arrays. This ensures attribute data is properly typed.
219-246: LGTM! Pragmatic approach to attribute parsing.The single constructor limitation is well-documented and reasonable given the complexity of overload resolution with implicit casts. The implementation correctly handles both constructor arguments and named properties.
248-372: LGTM! Well-structured code generation for nested types.The
ScopeandExtensionsimplementation elegantly handles nested type hierarchies and namespaces. The reversed storage of scopes simplifies the generation logic, and the formatted output with closing comments improves readability.SpacetimeDB/crates/bench/src/schemas.rs (3)
169-180: LGTM! Appropriate PRNG for benchmarking.The XorShift implementation is correct and suitable for deterministic benchmark data generation. The standard shift values (13, 7, 17) provide good pseudo-random properties for testing purposes.
218-260: LGTM! Well-designed data generation functions.The three data generation functions cover different benchmarking scenarios effectively:
- Sequential data for ordered access patterns
- Partly identical data for testing filter performance
- Random data for general benchmarking
The consideration for SQLite's u64 limitations is thoughtful.
332-347: LGTM! Clever deterministic name generation.The
nth_namefunction elegantly creates unique, deterministic names using modular arithmetic. The assertion verifying the inverse mapping formula ensures correctness, and the format provides good variation in string lengths for realistic benchmarking.SpacetimeDB/crates/bindings-csharp/BSATN.Codegen/Type.cs (6)
26-81: LGTM! Well-designed type representation system.The
TypeUsehierarchy effectively models different type categories with appropriate error handling. The rationale for inlining equality/hash operations instead of using runtime generics is well-documented and makes sense for IL2CPP performance.
157-214: LGTM! Robust array equality and hashing implementation.The
ArrayUseimplementation correctly handles all edge cases including null arrays and length mismatches. The use ofSystem.HashCodefor combining element hashes follows best practices.
332-400: LGTM! Sophisticated handling of partial types and TaggedEnums.The constructor correctly handles the nuanced case where users may add fields in separate partial declarations. The clear documentation with examples helps future maintainers understand this behavior. The TaggedEnum validation ensures proper usage.
427-493: LGTM! Clever resolution of C# namespace conflicts.The sum type generation elegantly handles C#'s namespace rules by appending underscores to field names. The generated pattern matching code for serialization and hashing is clean and efficient.
663-696: LGTM! Appropriate enum validation for SATS compatibility.The enum validation correctly enforces SATS requirements:
- Contiguous values (no explicit assignments)
- Maximum 256 variants (fitting in a byte tag)
The error messages provide clear guidance to users.
591-639: LGTM! Comprehensive equality implementation.The generated equality methods correctly handle both value and reference types with proper null checking. The use of
#nullable enableensures correct nullable reference type annotations.SpacetimeDB/crates/bench/benches/generic.rs (2)
16-26: Appropriate allocator selection for benchmarking.Using jemalloc for non-MSVC and mimalloc for MSVC targets is a good choice for consistent benchmarking performance across platforms.
114-141: Well-designed custom timing harness.The implementation correctly separates preparation time from benchmark execution time, ensuring accurate measurements. Good use of Criterion's
iter_customAPI.SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/BSATN/Runtime.cs (6)
59-68: Good documentation of optimization constraints.The comment clearly explains why
RuntimeHelpers.GetUninitializedObjectcouldn't be used, which helps future maintainers understand the design decision.
197-220: Necessary duplication well-documented.The comment clearly explains why separate implementations are needed for reference and value types despite identical bodies. This is a good example of documenting technical constraints.
487-551: Excellent performance optimizations in collection implementations.The code shows good attention to performance:
- Pre-allocation of arrays and lists based on count
- Special-cased byte array handling
- Code reuse between related implementations
617-679: Comprehensive string escaping implementation.Excellent handling of escape sequences including Unicode control characters. The implementation correctly handles all standard C# escape sequences and Unicode characters.
686-714: Smart handling of large collections in debug output.Good defensive programming - limiting display to 16 elements (8 from start, 8 from end) prevents UI freezing when accidentally printing large arrays. The ellipsis clearly indicates truncation.
402-406:⚠️ Potential issuePotential issue with signed 128-bit integer serialization.
Casting to
longin the Write method may cause issues with the bit representation of large values. Consider using unsigned operations for consistent bit manipulation.public void Write(BinaryWriter writer, Int128 value) { - writer.Write((long)value); - writer.Write((long)(value >> 64)); + writer.Write((ulong)(value & 0xFFFFFFFFFFFFFFFF)); + writer.Write((ulong)((value >> 64) & 0xFFFFFFFFFFFFFFFF)); }Likely an incorrect or invalid review comment.
SpacetimeDB/crates/bindings-csharp/Codegen.Tests/fixtures/diag/Lib.cs (5)
5-12: Good coverage of unsupported type scenarios.The test effectively covers various unsupported type categories including special types (DateTime), system types (Exception), unresolved types, and non-attributed enums.
22-281: Effective test for enum variant limit.Testing with exactly 257 variants (exceeding the 256 limit by 1) is a good boundary test for validating the enum size constraint.
301-329: Comprehensive reducer validation test coverage.Excellent coverage of reducer constraints including:
- Invalid return types
- Missing context parameters
- Duplicate reducer kinds and names
- Reserved name prefixes
- Cross-namespace name collision detection
364-372: Thorough index configuration validation tests.Good coverage of index validation scenarios:
- Index without columns
- Index with empty columns array
- Unknown column references
- Columns specified on field-level index
411-434: Well-structured ClientVisibilityFilter validation tests.Good use of pragma directives to test both enabled and disabled states of the unstable feature. Comprehensive coverage of field declaration constraints.
SpacetimeDB/crates/bindings-csharp/BSATN.Runtime/Builtins.cs (1)
82-85:⚠️ Potential issueFix string interpolation syntax error.
The error message has incorrect string interpolation syntax.
Apply this diff to fix the syntax error:
- $"Expected input string (\"{hex}\") to be of even length" + $"Expected input string (\"{hex}\") to have even length"Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
b9bbe793 Merge pull request #2 from clockworklabs/master c3cfadb2 SATS: Move some method bodies to be provided in 'Serializer' instead (#2921) 6a8cd7e9 Extract `object_pool` module from `PagePool` (#2920) 1166d68e Extract `spacetimedb-memory-usage` & Nix `bindings` dep on `spacetimedb-data-structures` (#2919) 6f5e33ab `update_database`: `is_unique` should only use the table's constraints (#2915) 06ba5293 Remove unused `get_column_by_field` (#2917) ef0b2569 core: Allow caller to decide whether or not IRQ cores should be reserved (#2913) c8716106 Record transaction metrics off the main thread (#2910) e4d5c18f Wrap websocket flush in timeout (#2908) 29a0df42 `tools/upgrade-version` updates the Change Date in `LICENSE.txt` (#2895) 9aebe31f Filter out dropped clients in the send worker (#2899) a7293a3c (perf) `send_one_computed_queries`: reuse aggregation `HashMap`s (#2909) 0c363518 Auto-migrate: Allow adding new variants at the tail (#2874) 47fd1684 durability: Fix task leak (#2875) 56f1f605 core: Allow adjusting CPU reservation pools (#2907) 022ca683 Fix query overwrites in the subscription manager (#2905) 28186d87 Track disconnects initiated by the client (#2893) c3afc171 Remove unnecessary `.clone()` in `pipelined.rs` (#2897) 67703b1f Bump C# versions to 1.2.1 (#2890) bc4fea20 `TypedIndex`: add special cases for `F{32,64}` (#2885) 971ae75b Improve serialization speed of enums without data in C# (#2762) 2f1d99ce Log when a websocket connection has ended (#2883) 9608f498 Speed up metadata update on unsubscribe (#2880) 8d250098 Fix two places we grab blocking locks on tokio runtime worker threads (#2881) 718712a8 Record the size of a client's outgoing message queue on drop (#2877) b63df787 Track metadata updates for unsubscribes (#2876) 4f493e4d Relax conditions for query pruning (#2873) 1318e7e1 `messages::serialize`: take/put buffers from/into a `SerializeBufferPool` (#2823) 27af02ce Use database identity not owner identity for metrics (#2868) da0757b5 Remove incorrect `const` qualifiers on two imports in `bindings.c` (#2862) 9f03b2c6 Fix an outdated docstring on `identity` and `try_identity` in the Rust SDK (#2871) c522c0f9 Prune more queries when evaluating subscription updates (#2855) 70df4fb0 Test subscription updates for dml (#2716) 228c761c Use a scope guard for decrementing queue length. (#2865) 1e3e8db9 Database-specific `Timestamp` route (#2864) 193c0d48 Add more subscription metrics (#2861) 2d756dbc Wrap `try_init_host` futures in `tokio::spawn` so they don't get cancelled (#2857) f4f698ef Always use identity when hashing queries from owners (#2859) 013e268e Fixed rename by using put instead of post (#2764) 12abd48a Send one-off responses through the broadcast queue. (#2853) 053fc6d9 Tidy up ModuleKind conversions a tiny bit (#2840) 4974676c client-api: Don't debug-print dropped outgoing Ws messages at info (#2819) a19bd458 Reserve cores 0 and 1 for the OS (#2851) 0866fd39 Add an HTTP route to get the current `Timestamp` (#2850) dc220ff3 fix: rust fmt git-subtree-dir: SpacetimeDB git-subtree-split: b9bbe793f39d88207a455c612f55f348d6cb44b3
…hboard-auth-workos
this flow is not completely finished yet!
this flow is not completely finished yet!
from now on all config will be defined in the global axonotes.config.ts file. new config files or variables can be just added and after running `bun run config:generate` it will update it in all places. todo: - watch doesnt work correctly yet - improve env variables in the dashboard
- Tailwindcss is now available in the desktop frontend - There were typescript problems with Uint8Array and Uint8Array<ArrayBuffer>. - Tokens are now generated based on the client_type attribute.
- The desktop app can now refresh its tokens in the background - The desktop app can now add, update and remove users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (11)
DEVELOPING.md (7)
1-4: Unify product naming in title ("AxonotesCore")Current: "axonotes Core". Recommend "AxonotesCore" for consistency with README and repo name.
-# axonotes Core: Developer Setup Guide +# AxonotesCore: Developer Setup Guide
10-22: Pin toolchain versions and document themAdd explicit versions (Rust toolchain, Node, Bun, Tauri deps) to reduce "works on my machine" drift. Consider rust-toolchain.toml and .node-version/.nvmrc, and note the Bun version.
## 1. Prerequisites -... (as-is) +... (as-is) + +Recommended versions +- Rust: stable (pinned via rust-toolchain.toml) +- Node.js: 20.x (via .nvmrc/.node-version) +- Bun: 1.x (set in bunfig.toml if applicable) +- Tauri prerequisites: per v2 docs for your OS
54-66: Fix Markdown list indentation (MD007)Sub-list items are indented by 4 spaces; markdownlint expects 2. Apply across the WorkOS steps and other nested lists.
- 1. Create a free WorkOS account: https://workos.com/signin - 2. Get your credentials from the dashboard: - - Navigate to "API Keys" section - - Copy your Client ID and API Key (starts with `sk_`) - 3. Configure redirect URI: - - Go to "Redirects" section in dashboard - - Add: `http://localhost:5173/auth/callback` - 4. Fill out your `.env.axogen` file with your credentials: + 1. Create a free WorkOS account: https://workos.com/signin + 2. Get your credentials from the dashboard: + - Navigate to "API Keys" section + - Copy your Client ID and API Key (starts with `sk_`) + 3. Configure redirect URI: + - Go to "Redirects" section in dashboard + - Add: `http://localhost:5173/auth/callback` + 4. Fill out your `.env.axogen` file with your credentials:Also applies to: 68-76, 90-92, 100-106
61-62: Align dev server port with WorkOS redirectRedirect uses http://localhost:5173. Pin the dashboard dev server to 5173 or note that the redirect must be updated if the port differs.
-**Dashboard (SvelteKit web app):** +**Dashboard (SvelteKit web app):** (served on port 5173 by default) ... bunx @axonotes/axogen run dash dev + +Note: If you change the dev server port, update the WorkOS redirect URI accordingly.Also applies to: 121-133
42-50: Provide a committed example env fileAdd .env.axogen.example with documented keys; keep .env.axogen in .gitignore.
-Create a `.env.axogen` file in your project root. +Create a `.env.axogen` file in your project root (use `.env.axogen.example` as a template).
154-160: Consistent casing: SpaceTimeDB vs SpacetimeDBBoth variants appear across docs. Pick one and apply consistently.
-- **`/SpacetimeDB`**: Git subtree of the SpacetimeDB engine (for custom modifications) +- **`/SpaceTimeDB`**: Git subtree of the SpaceTimeDB engine (for custom modifications)
51-67: WorkOS credentials: server-only key confirmed; redirect route exists — doc mismatch
- WORKOS_API_KEY is loaded server-side (dashboard/src/lib/server/workos.ts imports from $env/static/private); no PUBLIC_* env variables referencing WorkOS found.
- Redirect "/auth/callback" is the default (config/env.ts) and the route exists at dashboard/src/routes/auth/callback/+server.ts; DEVELOPING.md documents the same redirect URI.
- Minor: DEVELOPING.md says API key starts with
sk_but config/env.ts validatessk_test_— align the docs or the validation (DEVELOPING.md, config/env.ts).README.md (4)
10-10: Consistent casing for SpaceTimeDBUse one style across README and DEVELOPING.md (SpaceTimeDB vs SpacetimeDB).
-<strong>Core monorepo for the Axonotes Desktop application (Tauri/SvelteKit), web dashboard, and SpaceTimeDB Rust backend.</strong> +<strong>Core monorepo for the Axonotes Desktop application (Tauri/SvelteKit), web dashboard, and SpaceTimeDB (Rust) backend.</strong>
52-76: Sub-list indentation consistencySome sections use 4-space indents for sub-items; consider 2-space indents throughout to satisfy MD007.
89-107: Surface authentication provider in Tech StackSince this PR introduces WorkOS auth, add it explicitly.
- **Development Tools:** - Configuration Management: [Axogen](https://axonotes.github.io/axogen/) (our own TypeScript configuration system) - Package Manager: [Bun](https://bun.sh/) + - **Authentication:** + - Provider: [WorkOS](https://workos.com/) (SSO/Magic Link, redirects configured for local dev)
119-137: Tighten Quick Start step 5Either show the concrete commands or make step 5 a link-only note. Example below removes an empty code block.
-# 5. Start development environment (see DEVELOPING.md for full instructions) -``` - -For complete setup instructions, see [DEVELOPING.md](DEVELOPING.md). +# 5. Start development environment +# - Backend server: see "Start the Backend Server" in DEVELOPING.md +# - Publish modules: see "Publish Backend Modules" +# - Dashboard: see "Start the Frontend Applications" +``` +For complete setup instructions, see [DEVELOPING.md](DEVELOPING.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
DEVELOPING.md(1 hunks)README.md(4 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
DEVELOPING.md
54-54: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
61-61: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
63-63: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
64-64: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
68-68: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
69-69: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
70-70: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
75-75: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
90-90: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
91-91: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
92-92: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
94-94: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
95-95: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
97-97: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
98-98: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
100-100: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
101-101: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
103-103: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
105-105: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
106-106: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (4)
DEVELOPING.md (2)
150-153: Nice: linking to Axogen docsHelpful cross-reference; no changes needed.
70-91: Confirm @axonotes/axogen presence and verify CLI commands
- Found "@axonotes/axogen": "^0.5.7" in package.json (line 32).
- DEVELOPING.md calls the package via
bunx @axonotes/axogen ...(these are package CLI subcommands, not repo npm scripts). Verify the axogen package actually exposesrun setup,run sdb login, andgenerate— or add npm script wrappers in package.json for those commands so instructions match the repo.README.md (2)
141-151: Contact info looks good; minor copy edit optionalConsider "Watch this repository" -> "Watch this repository" (capitalization already consistent); no action required.
170-179: BSL Change Date verified — no action required
README.md shows "May 2, 2030" and LICENSE shows "Change Date: 2030-05-02" (they match).
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 84
♻️ Duplicate comments (6)
app/static/site.webmanifest (1)
12-22: Use relative icon paths and “any maskable” here as well.- "src": "/web-app-manifest-192x192.png", + "src": "web-app-manifest-192x192.png", "sizes": "192x192", "type": "image/png", - "purpose": "maskable" + "purpose": "any maskable" }, { - "src": "/web-app-manifest-512x512.png", + "src": "web-app-manifest-512x512.png", "sizes": "512x512", "type": "image/png", - "purpose": "maskable" + "purpose": "any maskable" }Quick checks:
#!/bin/bash set -euo pipefail # Confirm icons exist in both apps for d in app/static dashboard/static; do printf "\nChecking %s\n" "$d" for s in 192 512; do test -f "$d/web-app-manifest-${s}x${s}.png" && echo "OK $d ${s}x${s}" || echo "MISSING $d ${s}x${s}" done done # Ensure manifests are linked in HTML/layouts rg -nP -C2 '<link\s+rel=["\']manifest["\']\s+href=["\']/site\.webmanifest["\']' -g '!**/node_modules/**'dashboard/package.json (1)
47-63: Good fix: security‑sensitive versions pinned.Thanks for pinning WorkOS, argon2-browser, jose, and jsonwebtoken. This reduces supply-chain risk.
dashboard/src/app.html (1)
5-5: Add legacy favicon fallback.Include .ico for broader coverage.
- <link rel="icon" href="%sveltekit.assets%/favicon.svg" /> + <link rel="icon" type="image/svg+xml" href="%sveltekit.assets%/favicon.svg" /> + <link rel="icon" href="%sveltekit.assets%/favicon.ico" type="image/x-icon" />dashboard/src/lib/components/AuthButton.svelte (1)
1-3: Add CSRF protection using SvelteKit enhance on the logout formMatches prior feedback; enhances security and progressive enhancement.
<script lang="ts"> import {page} from "$app/state"; import {lockVault} from "$lib/client/cryptoStore"; + import {enhance} from "$app/forms"; @@ -{#if page.data.user} - <form action="/auth/logout" method="POST"> +{#if page.data.user} + <form action="/auth/logout" method="POST" use:enhance> <button type="submit" class="btn preset-filled-primary-500" on:click|preventDefault={handleLogout} > Logout </button> </form>Also applies to: 32-40
dashboard/src/routes/auth/setup/+page.svelte (2)
136-143: Clear master password and consider minimizing lifetime of secrets.Good that you clear masterPassword. Consider moving zeroization of buffers into finally as well when you restructure variables to outer scope; right now only masterPassword is cleared on error paths.
I can propose a scoped refactor that declares sensitive buffers outside the try for consistent cleanup—want that?
128-136: Zeroize sensitive material after use.Clear key bytes and reduce copies of the mnemonic in memory.
await setAndStoreVaultKeys(keysToStore); // 9. Show the user their recovery phrase generatedPassphrase = backupPassphrase; + // Clear plaintext private keys and temporary buffers + try { + rsaKeys.privateKey = ""; + ed25519Keys.privateKey = ""; + privateKeyBytes.fill(0); + privateSigningKeyBytes.fill(0); + passwordHash.fill(0); + backupAesKey.fill(0); + } catch {} + // Avoid duplicate copies of the passphrase in memory + // (generatedPassphrase remains for UI) + // @ts-expect-error reassign to clear + backupPassphrase = "";
🧹 Nitpick comments (102)
dashboard/src/types/argon2-browser.d.ts (1)
1-4: Prefer pure re-export in ambient module to avoid value imports in .d.tsUse a re-export pattern that doesn’t introduce value-level imports inside a declaration file and preserves both default and named exports.
declare module "argon2-browser/dist/argon2-bundled.min" { - import argon2 from "argon2-browser"; - export default argon2; + export * from "argon2-browser"; + export { default } from "argon2-browser"; }dashboard/static/robots.txt (1)
3-3: Consider disallowing auth endpoints from indexing.Prevents login/device/OAuth callback pages from being crawled.
Apply:
User-agent: * +Disallow: /auth/ +Disallow: /oauth/ Sitemap: https://axonotes.ch/sitemap.xmlAdjust paths to your actual routes (e.g., /auth/callback).
dashboard/static/site.webmanifest (2)
5-7: Add explicit scope/id to stabilize PWA identity and avoid Lighthouse warnings.Helps installs be recognized consistently and keeps start_url within scope.
"start_url": "/", + "scope": "/", + "id": "/", "display": "standalone",
12-22: Make icon src paths relative and broaden purpose for wider UA compatibility.Absolute “/…” breaks when the app is hosted under a subpath; “any maskable” covers both display modes.
- "src": "/web-app-manifest-192x192.png", + "src": "web-app-manifest-192x192.png", "sizes": "192x192", "type": "image/png", - "purpose": "maskable" + "purpose": "any maskable" }, { - "src": "/web-app-manifest-512x512.png", + "src": "web-app-manifest-512x512.png", "sizes": "512x512", "type": "image/png", - "purpose": "maskable" + "purpose": "any maskable" }app/static/site.webmanifest (1)
5-7: Mirror scope/id additions here to keep both manifests consistent."start_url": "/", + "scope": "/", + "id": "/", "display": "standalone",server/rustfmt.toml (1)
1-4: Consistent rustfmt config; consider de-duplication at repo root.Looks good. To avoid drift between crates, consider a single rustfmt.toml at the monorepo root if
cargo fmt --allis used from there.app/src-tauri/rustfmt.toml (1)
1-4: Mirror config is fine; optional: centralize formatting.Same note as server: a single top-level rustfmt.toml can reduce duplication.
app/src-tauri/build.rs (1)
1-3: Ensure rebuild on capability changes.Tauri’s build script won’t rerun for JSON edits unless watched. Add a rerun hint (and optional semicolon for style).
fn main() { - tauri_build::build() + tauri_build::build(); + // Re-run build if capabilities change + println!("cargo:rerun-if-changed=capabilities"); }.gitignore (1)
35-40: Add Rust target; confirm ignoring bin is intended.
- Recommend ignoring Cargo build output.
- Verify that ignoring repo-level bin won’t hide needed scripts/binaries.
# --- Build outputs --- bin + +# --- Rust --- +targetdashboard/README.md (1)
1-39: Replace scaffold boilerplate with project-specific instructions.This is the generic Svelte “sv” template. Swap in Axonotes Dashboard setup/run commands (workspace scripts, env vars, auth callbacks, Node version) to prevent confusion.
app/.gitignore (1)
1-10: Ignore a few common dev artifacts.Optional additions for smoother DX (coverage, eslint cache, Playwright).
.env.* !.env.example vite.config.js.timestamp-* vite.config.ts.timestamp-* + +# Tests / coverage +coverage +playwright-report +test-results + +# Lint cache +.eslintcacheapp/README.md (1)
1-8: Add a Quickstart block for dev/build.Make it immediately actionable for new contributors.
## Recommended IDE Setup [VS Code](https://code.visualstudio.com/) + [Svelte](https://marketplace.visualstudio.com/items?itemName=svelte.svelte-vscode) + [Tauri](https://marketplace.visualstudio.com/items?itemName=tauri-apps.tauri-vscode) + [rust-analyzer](https://marketplace.visualstudio.com/items?itemName=rust-lang.rust-analyzer). + +## Quickstart +```bash +# in app/ +pnpm i # or npm/yarn +pnpm dev # start SvelteKit +pnpm tauri dev # desktop (requires Rust + Tauri prerequisites) +```dashboard/vite.config.ts (1)
1-9: Plugin order is fine; consider client-only polyfills.To avoid SSR bloat, consider applying polyfills only on client if the plugin supports
apply: 'client'.- nodePolyfills({ + nodePolyfills({ + // apply: 'client', // if supported by the pluginapp/package.json (1)
15-36: Move Tailwind and its Vite plugin to devDependencies.They’re build-time only; keeping them out of runtime dependencies reduces install size.
"dependencies": { - "@lucide/svelte": "^0.525.0", - "@tailwindcss/vite": "^4.1.11", + "@lucide/svelte": "^0.525.0", "@tauri-apps/api": "^2", "@tauri-apps/plugin-deep-link": "~2", "@tauri-apps/plugin-opener": "^2", "@tauri-apps/plugin-store": "~2", - "tailwindcss": "^4.1.11" + // runtime deps only }, "devDependencies": { + "@tailwindcss/vite": "^4.1.11", + "tailwindcss": "^4.1.11", "@skeletonlabs/skeleton": "^3.1.4", "@skeletonlabs/skeleton-svelte": "^1.2.4", "@sveltejs/adapter-static": "^3.0.6",.prettierignore (2)
2-2: Verify directory case matches actual path.If the repo folder is named differently (case-sensitive filesystems),
SpacetimeDB/may not match. Consider confirming or using a glob (e.g.,spacetime*/).
23-27: Add trailing newline.Minor POSIX style nit to end files with a newline.
bun.lockb +config/jwt.ts (1)
3-21: Optional: expose JWK derivation for JWKS.If
.well-known/jwks.jsonis served, provide a helper to convertpublicKeyPEM → JWK (kty: "EC", crv: "P-256", x, y, alg: "ES256", kid). I can add this if desired.app/src-tauri/tauri.conf.json (1)
3-5: Minor: align productName with window title.Consider
productName: "Axonotes"for consistency.- "productName": "app", + "productName": "Axonotes",dashboard/package.json (1)
16-46: Move @types/argon2-browser to devDependencies.Type packages are compile-time only and shouldn’t ship as runtime deps.
Apply:
@@ "devDependencies": { + "@types/argon2-browser": "^1.18.4", @@ }, @@ "dependencies": { - "@types/argon2-browser": "^1.18.4",Also applies to: 47-63
app/svelte.config.js (1)
10-15: Enable full SSG by default.If the app is entirely static, make prerender default explicit.
kit: { adapter: adapter(), + prerender: { default: true },dashboard/axonotes-theme.css (1)
1-205: Add fallback tokens for browsers without OKLCH.OKLCH coverage is good but not universal; provide @supports fallback to sRGB to avoid invisible colors on older engines.
Example scaffold:
@supports not (color: oklch(50% 0 0)) { [data-theme="axonotes-theme"] { --color-primary-500: #635bff; /* pick close sRGB */ --color-surface-50: #f7f7fa; --color-surface-900: #22232a; /* add the handful of tokens actually used in UI */ } }dashboard/src/app.html (1)
6-6: Advertise color schemes to the UA.Helps form controls and built-ins pick correct palette.
<meta name="viewport" content="width=device-width, initial-scale=1" /> + <meta name="color-scheme" content="light dark" />eslint.config.js (3)
32-35: Use the type‑checked TS preset.You already enable
project: true; switch to the type‑aware config for stronger checks.- extends: [...tseslint.configs.recommended], + extends: [...tseslint.configs.recommendedTypeChecked],
20-28: Scope globals per target to avoid Node APIs in browser code.Applying both browser and node globals everywhere can mask accidental Node usage in the dashboard. Create targeted blocks (e.g., limit Node to server/scripts).
Example additional block:
+{ + files: ["dashboard/**/*.ts", "dashboard/**/*.svelte", "dashboard/**/*.js"], + languageOptions: { globals: { ...globals.browser } } +}, +{ + files: ["server/**/*.{ts,js}", "scripts/**/*.{ts,js}"], + languageOptions: { globals: { ...globals.node } } +},
10-16: Ignore generated dashboard bindings to speed up linting.They’re already eslint-disabled, but excluding the dir saves time.
ignores: [ "**/build/**", "**/dist/**", "**/.svelte-kit/**", "**/SpacetimeDB/**", "**/bin/**", + "dashboard/src/lib/module_bindings/**", ],server/src/errors.rs (1)
1-10: Map upstream errors viaFromfor ergonomic?usage.Implement
From<base64::DecodeError>and any crypto library errors into your enums to avoid lossy string wrapping.I can add concrete
Fromimpls once you confirm which crypto crates are used (e.g.,base64,ed25519-dalek,pkcs8).dashboard/src/lib/actions/copy.ts (2)
46-67: Dispatch copy events (and keep existing visual feedback).Emitting events lets callers show toasts or analytics without relying on text mutation.
try { await navigator.clipboard.writeText(params.text); // Provide visual feedback const originalText = params.defaultText || node.textContent; const successText = params.successText || "Copied!"; const duration = params.duration || 2000; node.textContent = successText; + node.dispatchEvent(new CustomEvent("copy:success", { detail: { text: params.text } })); // Revert to original text after a delay clearTimeout(timer); timer = window.setTimeout(() => { node.textContent = originalText; }, duration); } catch (err) { console.error("Failed to copy text: ", err); + node.dispatchEvent(new CustomEvent("copy:error", { detail: err })); }
46-67: Optional: add non‑secure‑context fallback.Clipboard API requires HTTPS/Tauri; consider a textarea fallback for dev/legacy contexts.
async function writeClipboard(text: string) { if (navigator.clipboard?.writeText) return navigator.clipboard.writeText(text); const ta = document.createElement("textarea"); ta.value = text; ta.style.position = "fixed"; ta.style.opacity = "0"; document.body.appendChild(ta); ta.select(); try { document.execCommand("copy"); } finally { document.body.removeChild(ta); } }And replace
navigator.clipboard.writeText(params.text)withwriteClipboard(params.text).DEVELOPING.md (2)
51-67: Clarify WorkOS envs and keep secrets guidance front-and-center.Recommend explicitly documenting WORKOS_REDIRECT_URI in the sample block to match the redirect configured in the dashboard.
Apply:
4. Fill out your `.env.axogen` file with your credentials: ```env WORKOS_CLIENT_ID="your_client_id_here" WORKOS_API_KEY="your_api_key_here" + WORKOS_REDIRECT_URI="http://localhost:5173/auth/callback" ```
141-148: Small DX tweak: remind to republish after backend changes.You already note this below; consider adding a single-line reminder here that frontend codegen depends on publish.
README.md (5)
10-10: Standardize brand casing: “SpacetimeDB”.Use “SpacetimeDB” consistently.
- <strong>Core monorepo for the Axonotes Desktop application (Tauri/SvelteKit), web dashboard, and SpaceTimeDB Rust backend.</strong> + <strong>Core monorepo for the Axonotes Desktop application (Tauri/SvelteKit), web dashboard, and SpacetimeDB Rust backend.</strong>
72-75: Standardize brand casing in section header.- - Git subtree of the SpaceTimeDB engine for custom modifications. + - Git subtree of the SpacetimeDB engine for custom modifications.
97-99: Standardize brand casing in tech stack.- - Platform: [SpaceTimeDB](https://spacetimedb.com/) + - Platform: [SpacetimeDB](https://spacetimedb.com/)
168-168: Fix version inconsistency.README shows 0.0.0 while package.json is 0.0.1.
-AxonotesCore (Version 0.0.0) is licensed under the **Business Source License 1.1 (BSL 1.1)**. +AxonotesCore (Version 0.0.1) is licensed under the **Business Source License 1.1 (BSL 1.1)**.
52-58: Resolve markdownlint MD007 (unordered list indentation).Dedent nested list items from 4 spaces to 2 spaces.
- - The Axonotes web dashboard application. - - Built with **[SvelteKit](https://kit.svelte.dev/)** for the web interface. + - The Axonotes web dashboard application. + - Built with **[SvelteKit](https://kit.svelte.dev/)** for the web interface. ... - - Framework: [Tauri](https://tauri.app/) - - UI: [SvelteKit](https://kit.svelte.dev/) + - Framework: [Tauri](https://tauri.app/) + - UI: [SvelteKit](https://kit.svelte.dev/) ... - - You **CAN** copy, modify, create derivative works, and redistribute the software. - - You **CAN** use it for non-production purposes. + - You **CAN** copy, modify, create derivative works, and redistribute the software. + - You **CAN** use it for non-production purposes.Also applies to: 89-107, 171-179
package.json (1)
6-8: Optional: add common scripts for DX.A simple “lint” and “format” wrapper helps CI and local runs.
"scripts": { - "prepare": "husky" + "prepare": "husky", + "format": "prettier --write .", + "lint": "eslint ." },app/src-tauri/src/user_profile/auth/jwk.rs (1)
58-67: Increase user_id entropy; document truncation.16 base64url chars ≈ 12 bytes (96 bits). Bump to 22 chars (128-bit) for safer uniqueness.
- BASE64_URL_SAFE_NO_PAD.encode(digest)[..16].to_string() + BASE64_URL_SAFE_NO_PAD.encode(digest)[..22].to_string()app/src-tauri/src/user_profile/auth/client.rs (3)
9-15: Unused struct.
DeviceAuthRequestisn’t used. Remove or use it instart_device_flowfor clarity.
108-151: Polling contract differences: confirm endpoint/method.GET on
/oauth/device/tokenreturning astatusfield isn’t standard RFC 8628 behavior (which expects POST token polling that returnsauthorization_pendingerrors). If this is a provider-specific extension, ignore. Otherwise, align to spec or add a comment.
214-217: Replace base64::prelude::BASE64_URL_SAFE_NO_PAD with general_purpose::URL_SAFE_NO_PADSome base64 versions don't expose the prelude constants; use the stable engine path (Cargo.toml shows base64 = "0.22.1").
Diff (apply to client.rs):
-use base64::Engine; +use base64::{engine::general_purpose, Engine as _}; @@ - base64::prelude::BASE64_URL_SAFE_NO_PAD.encode(digest) + general_purpose::URL_SAFE_NO_PAD.encode(digest)Also apply the same import + call change in app/src-tauri/src/user_profile/auth/jwk.rs (around lines 61 and 66) where BASE64_URL_SAFE_NO_PAD is used.
app/axonotes-theme.css (2)
25-29: Anchor accessibility: consider visible focus and underline by default.For better a11y, default to underlined links and show a clear focus state.
- --anchor-text-decoration: none; - --anchor-text-decoration-hover: underline; - --anchor-text-decoration-active: none; - --anchor-text-decoration-focus: none; + --anchor-text-decoration: underline; + --anchor-text-decoration-hover: underline; + --anchor-text-decoration-active: underline; + --anchor-text-decoration-focus: underline; + --anchor-underline-offset: 2px;
37-205: OKLCH-only tokens: add pragmatic fallbacks for older WebViews.Some embedded WebViews lag OKLCH. Where these tokens are consumed (not here), set a safe sRGB fallback before the var, e.g.,
color: #111; color: var(--base-font-color);. Consider an@supports (color: oklch(0% 0 0)) { ... }block to switch to OKLCH-only styles.dashboard/src/routes/dashboard/+page.server.ts (1)
1-6: Optional: add typing for load.Improves DX and catches mistakes.
-import {redirect} from "@sveltejs/kit"; +import { redirect, type PageServerLoad } from "@sveltejs/kit"; @@ -export async function load({locals}) { +export const load: PageServerLoad = async ({ locals }) => { @@ -} +};app/src-tauri/src/user_profile/auth/mod.rs (1)
1-5: Expose the primary type ergonomically via re-export.If
AuthManageris the main entrypoint, re-exporting it frommod.rssimplifies imports across the crate without widening module visibility.mod client; pub(crate) mod manager; mod jwk; +pub(crate) use manager::AuthManager;dashboard/src/routes/auth/login/+server.ts (2)
1-3: Confirm env source is correct for your deploy model.
$env/static/privateis baked at build time. If you need per-environment (runtime) values without rebuilding, prefer$env/dynamic/private. Otherwise, keep static for immutability.Would you like a patch to switch to
$env/dynamic/privatewith validation?
5-13: Validate the redirect URI and WorkOS config early.Add a guard that fails fast if
WORKOS_REDIRECT_URIorworkosClientIdis missing/malformed to avoid producing broken redirects.I can provide a small validation helper if you want.
dashboard/src/routes/+layout.server.ts (2)
1-5: Type the loader for safety and editor help.Add
LayoutServerLoadtyping; current function doesn't needasync.-export async function load({locals}) { - return { - user: locals.user, - }; -} +import type { LayoutServerLoad } from "./$types"; + +export const load: LayoutServerLoad = ({ locals }) => { + return { user: locals.user }; +};
1-5: Avoid leaking sensitive fields fromlocals.user.If
locals.usercontains tokens or PII, whitelist fields before exposing to the client (e.g.,id,name).Want a patch that safely picks fields and returns
nullwhen unauthenticated?app/src/app.html (1)
3-13: Advertise supported color schemes to the UA.Helps native form controls and UA styling respect dark/light properly.
<meta charset="utf-8" /> <link rel="icon" href="%sveltekit.assets%/favicon.svg" /> <meta name="viewport" content="width=device-width, initial-scale=1" /> + <meta name="color-scheme" content="light dark" /> <title>Axonotes</title> %sveltekit.head%dashboard/src/routes/auth/device/success/+page.svelte (1)
10-11: Mark decorative icon as hidden from assistive tech.Avoids redundant announcements; keep heading as the primary label.
- <CheckCircle class="text-success-500 h-16 w-16" /> + <CheckCircle class="text-success-500 h-16 w-16" aria-hidden="true" />dashboard/src/routes/auth/logout/+server.ts (2)
26-26: Use 303 after POST to prevent re-POST on reloadSwitch redirect to 303 See Other.
- throw redirect(302, "/"); + throw redirect(303, "/");
22-24: Ensure cookie deletions match original attributes (path/domain)
If these cookies were set with a custom domain, deletion may fail unless the domain matches. Consider centralizing cookie options (path, optional domain) and reusing here.dashboard/src/routes/dashboard/+page.svelte (2)
1-3: Type the page data for stronger safetyUse SvelteKit’s generated PageData type.
<script lang="ts"> - export let data; + import type { PageData } from "./$types"; + export let data: PageData; </script>
5-8: Guard against missing user to avoid runtime errorsAdd a simple conditional to prevent accessing properties on null/undefined.
-<h1 class="text-xl">Welcome to your Dashboard, {data.user.firstName}!</h1> -<p>You are logged in with the email: {data.user.email}</p> - -<pre>{JSON.stringify(data.user, null, 2)}</pre> +{#if data?.user} + <h1 class="text-xl">Welcome to your Dashboard, {data.user.firstName}!</h1> + <p>You are logged in with the email: {data.user.email}</p> + <pre>{JSON.stringify(data.user, null, 2)}</pre> +{:else} + <p>Loading…</p> +{/if}dashboard/src/routes/+layout.svelte (1)
6-7: Confirm Svelte 5 usage (children/$props and {@render})This layout uses Svelte 5 APIs. If the project is on Svelte 4, replace with a slot.
Svelte 4 fallback:
<!-- remove $props()/children --> <main class="container mx-auto flex-grow p-4 sm:p-6 lg:p-8"> <slot /> </main>Also applies to: 19-21
dashboard/src/routes/auth/device/+page.svelte (1)
59-121: DRY the form enhance logic and show consistent feedback.Extract a small
const withLoading = ...helper to reuse across forms and consider showing a spinner for “Use Other Account” for consistency.Also applies to: 123-156
app/src/lib/components/LightSwitch.svelte (1)
6-9: Prefer system color-scheme when no saved modeFallback to prefers-color-scheme for a better first-run experience.
- $effect(() => { - const mode = localStorage.getItem("mode") || "light"; + $effect(() => { + const stored = localStorage.getItem("mode"); + const mode = + stored ?? + (window.matchMedia && + window.matchMedia("(prefers-color-scheme: dark)").matches + ? "dark" + : "light"); checked = mode === "dark"; });dashboard/src/routes/auth/device/error/+page.svelte (1)
72-74: Prevent indexing of error pagesAdd noindex to avoid search engines indexing transient auth errors.
<svelte:head> <title>Device Authorization Error</title> + <meta name="robots" content="noindex" /> </svelte:head>dashboard/src/lib/components/AuthButton.svelte (1)
13-19: Avoid noisy console logs in productionReduce auth-related console noise or guard behind dev checks.
- console.log("Locking client-side vault before redirecting..."); + // if (import.meta.env.DEV) console.debug("Locking vault before logout..."); @@ - console.log("Vault locked. Proceeding with server logout."); + // if (import.meta.env.DEV) console.debug("Vault locked. Proceeding with server logout.");dashboard/src/lib/components/LightSwitch.svelte (2)
6-9: Prefer system color-scheme when no saved modeMirror the app component’s behavior for consistent theming.
- $effect(() => { - const mode = localStorage.getItem("mode") || "light"; + $effect(() => { + const stored = localStorage.getItem("mode"); + const mode = + stored ?? + (window.matchMedia && + window.matchMedia("(prefers-color-scheme: dark)").matches + ? "dark" + : "light"); checked = mode === "dark"; });
1-60: DRY: deduplicate LightSwitch implementationsConsider extracting a shared LightSwitch to a common library used by both app and dashboard to avoid divergence.
dashboard/src/lib/client/persistentKeyStore.ts (2)
6-6: Single fixed KEY_ID limits multi-account use.Namespace by user/tenant to avoid overwriting keys when switching accounts on the same origin.
-const KEY_ID = "user-private-key"; +const KEY_ID_PREFIX = "user-private-key:"; +const keyIdFor = (userId: string) => `${KEY_ID_PREFIX}${userId}`;
25-25: Reduce production console noise.Gate logs behind a debug flag or remove in production builds.
Also applies to: 36-36, 48-48
dashboard/src/hooks.server.ts (1)
69-72: Consider path prefix or regex matching for allowedPaths.Exact path matching is brittle if routes get trailing slashes or versioned paths.
dashboard/src/routes/auth/device/callback/+server.ts (2)
57-64: Reconsider placing access token in a non-HttpOnly cookie.If XSS occurs, the token is exfiltrated. If you must expose it to JS, minimize lifetime and scope.
- cookies.set(PUBLIC_ACCESS_TOKEN_COOKIE_NAME, tempAccessToken, { + cookies.set(PUBLIC_ACCESS_TOKEN_COOKIE_NAME, tempAccessToken, { path: "/", httpOnly: false, - secure: !IN_DEVELOPMENT, - maxAge: Math.floor(accessTokenMs / 1000), - sameSite: "lax", + secure: !IN_DEVELOPMENT, + maxAge: Math.min(Math.floor(accessTokenMs / 1000), 60 * 5), // cap at 5m + sameSite: "strict", });Also applies to: 65-72
80-83: Avoid logging sensitive auth errors verbatim.Sanitize error output to prevent accidental token leakage in logs.
- } catch (error) { - console.error("Device auth callback failed:", error); + } catch (error) { + console.error("Device auth callback failed"); throw redirect(302, "/auth/device/error?error=auth_failed"); }dashboard/src/lib/server/workos.ts (1)
13-17: Optional: Add JWKS network timeouts/cooldown to avoid slow boots and stampedes.jose supports timeouts and cooldowns on JWKS fetch. Consider:
- const jwksUrl = workos.userManagement.getJwksUrl(WORKOS_CLIENT_ID); - return createRemoteJWKSet(new URL(jwksUrl)); + const jwksUrl = workos.userManagement.getJwksUrl(WORKOS_CLIENT_ID); + return createRemoteJWKSet(new URL(jwksUrl), { + timeoutDuration: 5_000, // 5s fetch timeout + cooldownDuration: 60_000 // 60s key miss cooldown + });dashboard/src/routes/auth/callback/+server.ts (2)
56-63: Optional: Prefer httpOnly for access token to limit XSS blast radius.If feasible, move access-token consumption server-side and set httpOnly: true.
- cookies.set(PUBLIC_ACCESS_TOKEN_COOKIE_NAME, axonotesAccessToken, { + cookies.set(PUBLIC_ACCESS_TOKEN_COOKIE_NAME, axonotesAccessToken, { path: "/", - httpOnly: false, + httpOnly: true, secure: !IN_DEVELOPMENT, maxAge: Math.floor(accessTokenMs / 1000), sameSite: "lax", });
79-81: Avoid logging secrets.Ensure errors from the SDK don’t include tokens or PII before logging; otherwise mask/sanitize.
dashboard/src/routes/auth/recover/+page.svelte (1)
184-191: UX/security nits: disable autocorrect/capitalize and set autocomplete.This reduces input mishaps and browser leakage.
- <textarea + <textarea id="passphrase" bind:value={passphrase} rows="3" class="textarea p-2" placeholder="word1 word2 word3..." + autocomplete="off" + autocapitalize="off" + autocorrect="off" + spellcheck="false" required ></textarea> ... - <input + <input id="newPassword" type="password" bind:value={newPassword} class="input mb-1" + autocomplete="new-password" required /> ... - <input + <input id="confirmPassword" type="password" bind:value={confirmPassword} class="input" + autocomplete="new-password" required />Also applies to: 215-221, 227-233
app/src-tauri/Cargo.toml (2)
27-30: Explicit TLS backend for reqwest.For predictable, cross-platform TLS and smaller builds, explicitly pick a TLS backend (e.g., rustls) and disable defaults.
-reqwest = {version = "0.12.22", features = ["json"] } +reqwest = { version = "0.12.22", default-features = false, features = ["json", "rustls-tls"] }
30-31: Trim Tokio features to what you use.
features = ["full"]pulls a lot. Consider narrowing for smaller binaries.Example minimal set (adjust to actual usage):
-tokio = {version = "1.47.1", features = ["full"] } +tokio = { version = "1.47.1", features = ["rt-multi-thread", "macros", "time", "sync", "fs"] }app/src-tauri/src/user_profile/mod.rs (1)
31-33: Return errors, not bool, from refresh.A boolean hides failure reasons and makes retries/backoff harder. Prefer Result<(), E>.
- pub async fn refresh_tokens(&mut self) -> bool { - self.auth_manager.refresh_tokens().await + pub async fn refresh_tokens(&mut self) -> anyhow::Result<()> { + self.auth_manager.refresh_tokens().await }dashboard/src/lib/server/rate-limiter.ts (3)
94-105: Remove unnecessary async from checkLimit.No awaits inside; avoid extra Promise machinery.
- async checkLimit( + checkLimit( identifier: string, operation: string, customConfig?: Partial<RateLimitConfig> - ): Promise<{ + ): { allowed: boolean; retryAfter?: number; shouldSlowDown?: boolean; - }> { + } { ... - return {allowed: true}; + return {allowed: true}; }Also applies to: 171-172
37-45: Clamp poll interval input.Prevent zero/negative values from breaking slow-down logic.
- setPollInterval(intervalSeconds: number) { - this.pollInterval = intervalSeconds; + setPollInterval(intervalSeconds: number) { + this.pollInterval = Math.max(1, Math.floor(intervalSeconds || 0)); }
32-42: In-memory limiter is per-instance only.NodeCache won’t protect you across multiple server instances or deploys. For production, back this with Redis/Memcached or a provider-native rate limit (e.g., CDN/WAF) and keep this as a local shield.
dashboard/src/routes/auth/setup/+page.svelte (1)
203-207: Form enhance is redundant with manual submit handling.You already preventDefault and handle submit; remove
use:enhanceunless you use progressive-enhancement callbacks.- <form - onsubmit={handleSetup} - use:enhance - class="flex flex-col" - > + <form onsubmit={handleSetup} class="flex flex-col">dashboard/src/lib/client/cryptoStore.ts (2)
23-30: Harden startup load with error handling
loadKey()rejections are silently dropped; add a.catchto avoid noisy unhandled promise rejections and aid diagnostics.if (browser) { - persistentKeyStore.loadKey().then((key) => { - if (key) { - console.log("Vault unlocked via persistent key on startup."); - set(key); - } - }); + persistentKeyStore + .loadKey() + .then((key) => { + if (key) { + console.log("Vault unlocked via persistent key on startup."); + set(key); + } + }) + .catch((err) => { + console.warn("Failed to load persisted vault keys on startup.", err); + }); }
84-99: Reduce key material residency in memoryZeroize transient buffers after import (e.g.,
privateKeyBytes,privateSigningKeyBytes,passwordHashif mutable) to minimize exposure in long-lived tabs.If you want, I can add a small
wipe(u8: Uint8Array)helper and apply it post-import.Also applies to: 100-107
dashboard/src/routes/auth/device/+page.server.ts (2)
105-109: Cookie deletion may fail without domainIf cookies were set with a domain attribute, deleting them without the same domain won’t work. Include
domain(and keeppath).- cookies.delete(PUBLIC_ACCESS_TOKEN_COOKIE_NAME, {path: "/"}); - cookies.delete(REFRESH_TOKEN_COOKIE_NAME, {path: "/"}); - cookies.delete(WORKOS_SESSION_ID_COOKIE_NAME, {path: "/"}); + const domain = new URL(JWT_ISSUER).hostname; + const opts = { path: "/", domain }; + cookies.delete(PUBLIC_ACCESS_TOKEN_COOKIE_NAME, opts); + cookies.delete(REFRESH_TOKEN_COOKIE_NAME, opts); + cookies.delete(WORKOS_SESSION_ID_COOKIE_NAME, opts);
31-41: VerifyverifyToken’s sync/async contractIf
verifyTokenis async, these calls must be awaited; otherwise the guards can be bypassed on promise truthiness.Would you like me to scan
$lib/server/jwt.tsand patch usage accordingly? I can provide a targeted diff once confirmed.Also applies to: 65-68
config/commands.ts (3)
49-56: User-facing help text: grammar fixTiny copy nit.
- "Run the database in memory and dont write to disk" + "Run the database in memory and don't write to disk"
203-206: Copy nit and UX: soften optional tool messageGrammar fix and clearer guidance.
- console.log( - `Its recommended to install ${name} from ${link}. But you can continue without it.` - ); + console.log( + `It's recommended to install ${name} from ${link}, but you can continue without it.` + );
184-186: Prefercwdover shellcdfor portabilityUsing
cdinside the command ties you to a shell. You already usecwdelsewhere—apply consistently.- await liveExec("cd dashboard && bun install"); - await liveExec("cd app && bun install"); + await liveExec("bun install", { cwd: "./dashboard" }); + await liveExec("bun install", { cwd: "./app" });Likewise:
- dev: "cd dashboard && bun run dev", - build: "cd dashboard && bun run build", - preview: "cd dashboard && bun run preview", - install: "cd dashboard && bun install", + dev: cmd({ exec: async () => liveExec("bun run dev", { cwd: "./dashboard" }) }), + build: cmd({ exec: async () => liveExec("bun run build", { cwd: "./dashboard" }) }), + preview: cmd({ exec: async () => liveExec("bun run preview", { cwd: "./dashboard" }) }), + install: cmd({ exec: async () => liveExec("bun install", { cwd: "./dashboard" }) }),- await liveExec( - "spacetime generate --lang typescript --out-dir dashboard/src/lib/module_bindings --project-path server" - ); + await liveExec("spacetime generate --lang typescript --out-dir src/lib/module_bindings --project-path ../server", { cwd: "./dashboard" }); @@ - await liveExec( - "spacetime generate --lang rust --out-dir app/src-tauri/src/module_bindings --project-path server" - ); + await liveExec("spacetime generate --lang rust --out-dir src-tauri/src/module_bindings --project-path ../server", { cwd: "./app" });Also applies to: 130-137, 223-231
app/src/routes/+page.svelte (2)
59-67: Race-safe updates when users are removed mid-waitIf a user is removed while
waitForAuthis in-flight, the mapper will reinsert an entry. Guard by bailing out if the user no longer exists.async function waitForAuth(userId: number) { try { let wait_response: { status: string; error_message: string | null; } = await invoke("wait_for_user_auth", { user_profile_id: userId, }); - // Update user status - users = users.map((user) => { + // Update user status (skip if user was removed) + const exists = users.some((u) => u.user_profile_id === userId); + if (!exists) return; + users = users.map((user) => { if (user.user_profile_id === userId) { if (wait_response.error_message) { return { ...user, status: "error", error: wait_response.error_message, }; } else { return {...user, status: "authenticated"}; } } return user; }); @@ } catch (e) { console.error("Error waiting for user auth:", e); // Update user status to error - users = users.map((user) => { + const exists = users.some((u) => u.user_profile_id === userId); + if (!exists) return; + users = users.map((user) => { if (user.user_profile_id === userId) { return {...user, status: "error", error: String(e)}; } return user; }); } }Also applies to: 68-83, 101-108
15-27: User feedback on early failuresWhen starting auth fails, the function returns silently. Consider surfacing a toast/inline error so the user knows what happened.
I can wire a minimal in-card error state if you want.
Also applies to: 31-37, 47-52
dashboard/src/lib/client/spacetime.ts (1)
26-26: Remove unused _connectionInstance.-let _connectionInstance: DbConnection | null = null;If you intend to expose/close it later, wire it into spacetime store or export a close() helper.
app/src-tauri/src/app_state/mod.rs (1)
5-5: Remove unused import.-use anyhow::Result;dashboard/src/lib/module_bindings/user_table.ts (1)
1-114: Auto-generated bindings look fine.Note:
removeOnUpdate’s parameter nameonRowis inconsistent witholdRow(harmless).app/src-tauri/src/user_profile/auth/manager.rs (1)
1-1: Remove unused import.-use std::error::Error;dashboard/src/lib/server/jwt.ts (3)
27-38: Guard against invalid lifetime strings.
ms()returnsundefinedon invalid input; surface a clear error.export function getTokenLifetimes(isDesktop: boolean = false) { - if (isDesktop) { + if (isDesktop) { return { - accessTokenMs: ms(DEFAULT_DESKTOP_ACCESS_LIFETIME), - refreshTokenMs: ms(DEFAULT_DESKTOP_REFRESH_LIFETIME), + accessTokenMs: ms(DEFAULT_DESKTOP_ACCESS_LIFETIME)!, + refreshTokenMs: ms(DEFAULT_DESKTOP_REFRESH_LIFETIME)!, }; } return { - accessTokenMs: ms(DEFAULT_WEB_ACCESS_LIFETIME), - refreshTokenMs: ms(DEFAULT_WEB_REFRESH_LIFETIME), + accessTokenMs: ms(DEFAULT_WEB_ACCESS_LIFETIME)!, + refreshTokenMs: ms(DEFAULT_WEB_REFRESH_LIFETIME)!, }; }At module init, validate the lifetimes with a descriptive throw if any
ms(...)result is falsy.Also applies to: 155-170
112-126: Minor: option names on generateAccess/Refresh are asymmetrical.Passing both lifetimes to each function is harmless but noisy. Consider aligning option types or removing unused fields.
Also applies to: 128-142
63-71: Consider setting standard claims via sign options.Set
subjectand optionallyaudienceinjwt.signto match verification constraints.dashboard/src/lib/client/crypto.ts (2)
9-20: Avoid O(n²) string concatenation in Base64 conversion.Use chunk array + join to reduce allocations.
export function arrayBufferToBase64(buffer: ArrayBufferLike): string { const bytes = new Uint8Array(buffer); const chunkSize = 0x8000; // Process in 32KB chunks - let binary = ""; + const chunks: string[] = []; for (let i = 0; i < bytes.length; i += chunkSize) { const chunk = bytes.subarray(i, i + chunkSize); - binary += String.fromCharCode(...chunk); + chunks.push(String.fromCharCode(...chunk)); } - return window.btoa(binary); + const binary = chunks.join(""); + return window.btoa(binary); }
1-3: Defer argon2 import to runtime to avoid SSR pitfalls and reduce initial bundle size.Top-level importing the WASM bundle can break SSR and increase startup cost.
-import argon2 from "argon2-browser/dist/argon2-bundled.min"; +// Defer argon2 import to function scope to avoid SSR issues.export async function hashPassword( password: string, salt: Uint8Array ): Promise<Uint8Array> { - const hashResult = await argon2.hash({ + const { default: argon2 } = await import("argon2-browser/dist/argon2-bundled.min"); + const hashResult = await argon2.hash({ pass: password, salt: salt, time: 3, mem: 128 * 1024, hashLen: 32, parallelism: 1, type: argon2.ArgonType.Argon2id, }); return hashResult.hash; }Please verify that this module is never imported in server-only contexts; if it is, gate calls with SvelteKit’s
browserflag.Also applies to: 132-147
dashboard/src/lib/server/validation-schemas.ts (2)
67-78: Redundant refine for PKCE presence.Both fields are already required by the schema.
- .refine( - (data) => { - // Both code_challenge and code_challenge_method must be present - const hasChallenge = !!data.code_challenge; - const hasMethod = !!data.code_challenge_method; - return hasChallenge && hasMethod; - }, - { - message: - "Both code_challenge and code_challenge_method are required", - } - ); + // PKCE fields are required above; additional refine not needed.
96-103: Harden size parsing and consider a slightly higher cap.Use base 10 for parseInt and consider 4KB to tolerate longer refresh tokens.
export const validateFormSize = (request: Request) => { const contentLength = request.headers.get("content-length"); - const MAX_FORM_SIZE = 1024; // 1KB max + const MAX_FORM_SIZE = 4096; // 4KB max - if (contentLength && parseInt(contentLength) > MAX_FORM_SIZE) { + if (contentLength && parseInt(contentLength, 10) > MAX_FORM_SIZE) { throw new Error("Request too large"); }If you must stay strict per your threat model, keep 1KB; otherwise 4KB avoids false rejects.
dashboard/src/lib/server/oauth.ts (2)
129-143: Persist scope (and optionally client_id) in the session and reflect it in TokenResponse.You accept
scope/client_idbut don’t store/use them; TokenResponse.scope is hardcoded.Apply:
- Add
scope?: string; client_id?: string;to DeviceFlowSession.- Store them in initiateDeviceFlow.
- Return the stored scope in exchangeDeviceToken.
I can prep a minimal diff touching the interface,
initiateDeviceFlow, andexchangeDeviceToken.
341-356: Dead code: cleanupExpiredSessions is never invoked.Either wire it to a timer or rely solely on NodeCache TTL.
- private cleanupExpiredSessions(): void { + // Option 1: keep and schedule; Option 2: remove if relying on NodeCache TTL + private cleanupExpiredSessions(): void {If you want scheduling, I can add a lightweight interval gated by an env flag.
app/src-tauri/src/module_bindings/update_encryption_keys_reducer.rs (2)
13-18: Consider binary-safe types for keys/salt.If feasible in your module schema, prefer bytes for encrypted keys/salt rather than
Stringto avoid base64 overhead and Unicode pitfalls; let the client handle base64 only at UI boundaries.
149-168: Set appropriate reducer call flags.If
update_encryption_keysdoesn’t mutate tables, set reducer call flags to a lighter mode (e.g., no full DB update) to reduce traffic. Coordinate with the TS side defaults.dashboard/src/lib/module_bindings/index.ts (1)
242-251: Right-size reducer update flags.Defaults are
"FullUpdate"for both reducers. If these reducers don’t modify tables, consider a lighter flag to avoid unnecessary cache diffs and websocket payloads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
- Remove duplicate permission and fix missing schema path - Fix lint-staged glob for Tauri Rust sources (subdirectories not covered). - Add OAuth2-recommended no-store headers (and Retry-After on 429). - Do not upcast client_type from refresh token to desktop. - Normalize user_code input (strip all dashes, uppercase) to reduce UX friction. - Apply the same normalization in validateUserCode and update regex.
|
@coderabbitai generate docstrings |
✅ Actions performedDocstrings generation started. |
Docstrings generation was requested by @ImGajeed76. * #27 (comment) The following files were modified: * `app/src-tauri/build.rs` * `app/src-tauri/src/app_state/mod.rs` * `app/src-tauri/src/lib.rs` * `app/src-tauri/src/main.rs` * `app/src-tauri/src/module_bindings/client_connected_reducer.rs` * `app/src-tauri/src/module_bindings/init_encryption_and_signing_reducer.rs` * `app/src-tauri/src/module_bindings/mod.rs` * `app/src-tauri/src/module_bindings/update_encryption_keys_reducer.rs` * `app/src-tauri/src/module_bindings/user_table.rs` * `app/src-tauri/src/user_profile/auth/client.rs` * `app/src-tauri/src/user_profile/auth/jwk.rs` * `app/src-tauri/src/user_profile/auth/manager.rs` * `app/src-tauri/src/user_profile/mod.rs` * `config/commands.ts` * `config/jwt.ts` * `dashboard/src/hooks.server.ts` * `dashboard/src/lib/actions/copy.ts` * `dashboard/src/lib/client/crypto.ts` * `dashboard/src/lib/client/cryptoStore.ts` * `dashboard/src/lib/client/persistentKeyStore.ts` * `dashboard/src/lib/client/spacetime.ts` * `dashboard/src/lib/module_bindings/client_connected_reducer.ts` * `dashboard/src/lib/module_bindings/init_encryption_and_signing_reducer.ts` * `dashboard/src/lib/module_bindings/update_encryption_keys_reducer.ts` * `dashboard/src/lib/module_bindings/user_type.ts` * `dashboard/src/lib/server/jwt.ts` * `dashboard/src/lib/server/validation-schemas.ts` * `dashboard/src/routes/+layout.server.ts` * `dashboard/src/routes/.well-known/jwks.json/+server.ts` * `dashboard/src/routes/.well-known/openid-configuration/+server.ts` * `dashboard/src/routes/api/auth/refresh/+server.ts` * `dashboard/src/routes/auth/callback/+server.ts` * `dashboard/src/routes/auth/device/+page.server.ts` * `dashboard/src/routes/auth/device/callback/+server.ts` * `dashboard/src/routes/auth/device/complete/+server.ts` * `dashboard/src/routes/auth/login/+server.ts` * `dashboard/src/routes/auth/logout/+server.ts` * `dashboard/src/routes/dashboard/+page.server.ts` * `dashboard/src/routes/oauth/device/authorize/+server.ts` * `dashboard/src/routes/oauth/device/token/+server.ts` * `dashboard/src/routes/oauth/token/+server.ts` * `server/src/errors.rs`
|
Note Generated docstrings for this pull request at #29 |
(to tired to write the description, I'm sure CodeRabbit will do a good job at describing it 💀)
Summary by CodeRabbit
New Features
UX / Design
Documentation
Chores