Conversation
- Add database initialization for Better Auth tables (user, session, account, verification, apikey) - Enable Better Auth admin and API key plugins in the backend - Implement Accounts settings UI with management sections for users, API keys, and OAuth providers - Add custom hooks and mutations for account management operations - Update OpenAPI path prefix to /api/v2/auth for better route organization
- Add Zed editor configuration for Biome and TypeScript support - Refactor authentication setup to expose a reusable `betterAuthPlugin` for Elysia - Update API entry point to use the new authentication plugin and reorder middleware initialization
- Add @dockstat/api path alias to tsconfig.json for both api and dockstat apps - Clean up unused import in elysia-plugins - Improve readability of getSession call in auth plugin - Export CommandResult type in docknode treaty
Implements a decorator-based routing system for Elysia JS with full TypeScript type safety via TypeBox schema extraction. Includes comprehensive documentation with examples and migration guides. Features: - Proper @controller() decorator pattern (fixes ts 1206 errors) - Type extraction utilities for compile-time safety - Support for @BodySchema, @QuerySchema, @ParamsSchema, etc. - Dual pattern support (context destructuring and parameter decorators) - typedRoute helper for functional style handlers - Extensive documentation on type safety approaches
The new auth package introduces database schemas and handlers for users, API tokens, and OIDC providers. The API application has been updated to use the new AuthHandler for managing users and API keys.
- Implement join, innerJoin, leftJoin, rightJoin, fullJoin, and crossJoin methods - Add generic type merging to allow IntelliSense for joined table columns - Support join conditions via simple column mapping or raw SQL expressions - Enable table aliasing for complex queries and self-joins - Update README documentation with join examples and migration notes - Add extensive type-safe JOIN unit tests in __test__/join-types.test.ts - Refactor QueryBuilder architecture to support chained result type transformations
…no-ticket] - Standardize log formatting across all query builders and database operations by adding bracketed prefixes (e.g., [INIT], [SELECT], [TRANSACTION]) for better traceability. - Enhance logging detail with operational context and result summaries in query-builder modules. - Refactor `BaseQueryBuilder` to include a `logWithTable` helper for consistent logging. - Update `transformFromDb` and `transformToDb` to provide more granular debug information during data serialization. - Correct boolean expectation in `join-types.test.ts` and update serialization test in `transformer.test.ts` to be more robust.
…ces for type-safe inference - Update join methods (join, innerJoin, leftJoin, etc.) to accept a `QueryBuilder` instance instead of a string table name. - Implement automatic parser merging for joined tables to ensure correct column transformation (e.g., boolean/date/json parsing) on joined data. - Update type-safe documentation and examples to remove generic type parameters in join calls. - Improve `BaseQueryBuilder` with public accessors for table names and parsers to facilitate cross-builder interactions. - Cleanup unused `tableParsers` state in the main DB class.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Its4Nik, your pull request is larger than the review limit of 150000 diff characters
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/sqlite-wrapper/src/utils/transformer.ts (1)
143-151:⚠️ Potential issue | 🟠 MajorHandle invalid date strings explicitly;
new Date()won't throw.The
new Date(value)call createsInvalid Dateobjects without throwing exceptions, so invalid date strings silently pass through the catch block and get stored with the "successfully transformed" marker. The try-catch is ineffective here.Proposed fix
- if (value !== null && value !== undefined && typeof value === "string") { - try { - transformed[columnKey] = new Date(value) - transformedColumns.push(`DATE:${columnKey}`) - logger.debug(`[FROM_DB] DATE column parsed ${value}`, columnKey) - } catch { - // Keep original value if DATE parsing fails - logger.warn(`[FROM_DB] Failed to parse DATE column Value: ${value}`, columnKey) - } - } + if (value !== null && value !== undefined && typeof value === "string") { + const parsed = new Date(value) + if (!Number.isNaN(parsed.getTime())) { + transformed[columnKey] = parsed + transformedColumns.push(`DATE:${columnKey}`) + logger.debug(`[FROM_DB] DATE column parsed`, columnKey) + } else { + // Keep original value if DATE parsing fails + logger.warn(`[FROM_DB] Failed to parse DATE column`, columnKey) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/utils/transformer.ts` around lines 143 - 151, The code assumes new Date(value) will throw on invalid input but it doesn't; replace the try/catch with explicit validation: create const parsed = new Date(value) and check if Number.isNaN(parsed.getTime()) before assigning to transformed[columnKey] and pushing `DATE:${columnKey}` to transformedColumns; if invalid, do not mutate transformed/transformedColumns and log a warning via logger.warn(`[FROM_DB] Failed to parse DATE column Value: ${value}`, columnKey). Ensure you reference the existing variables transformed, transformedColumns, columnKey, logger and value in the updated logic (in the same block where typeof value === "string" is checked).apps/api/src/elysia-plugins.ts (2)
5-13:⚠️ Potential issue | 🔴 Critical
OpenAPIdoes not exist in the imported module and will cause a startup failure.Line 5 imports
{ OpenAPI } from "./auth", butapps/api/src/auth/index.tsonly exportsAuthHandler. The usage at lines 12–13 (OpenAPI.components,OpenAPI.getPaths()) will fail at runtime. Either implementOpenAPIin the auth module or correct the import path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/elysia-plugins.ts` around lines 5 - 13, The code imports and uses OpenAPI (OpenAPI.components, OpenAPI.getPaths) but ./auth only exports AuthHandler, causing a runtime failure; fix by either exporting a proper OpenAPI object from the auth module (implement and export OpenAPI with components and getPaths) or change the import in the DockStatElysiaPlugins setup to the correct module that provides OpenAPI; update the import line and ensure the openapi({ documentation: { components: await OpenAPI.components, paths: await OpenAPI.getPaths() } }) call references the exported symbol (OpenAPI) or replace those calls with the correct functions/exports from the auth module (AuthHandler) or the actual OpenAPI provider so DockStatElysiaPlugins and openapi use valid symbols.
7-18:⚠️ Potential issue | 🔴 CriticalBroken import: OpenAPI is not exported from auth module.
Line 5 imports
OpenAPIfrom"./auth", but the auth module only exportsAuthHandler(from index.ts) andgetAuthTable(from db.ts). The OpenAPI export does not exist. This will cause a build or runtime error when elysia-plugins.ts attempts to useOpenAPI.componentsandOpenAPI.getPaths()on lines 12–13. The OpenAPI functionality needs to be either implemented in the auth module or imported from the correct location.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/elysia-plugins.ts` around lines 7 - 18, The import of OpenAPI used by DockStatElysiaPlugins (openapi({ documentation: { components: await OpenAPI.components, paths: await OpenAPI.getPaths() } })) is invalid because OpenAPI is not exported from the auth module; update the import so OpenAPI comes from the correct module that actually exports components/getPaths (or add an OpenAPI export from the auth module), then ensure the file imports that symbol instead of OpenAPI from "./auth"; verify DockStatElysiaPlugins and the openapi(...) call use the corrected OpenAPI symbol name and methods.packages/sqlite-wrapper/src/migration.ts (1)
248-354:⚠️ Potential issue | 🔴 CriticalRestore
foreign_keysin afinally.If any migration step throws, control exits before the re-enable block runs and the shared connection keeps
PRAGMA foreign_keys = OFF. That leaves every later write on this DB handle without FK enforcement.Suggested fix
const fkStatus = db.prepare("PRAGMA foreign_keys").get() as ForeignKeyStatus | null - if (fkStatus && fkStatus.foreign_keys === 1) { + const shouldRestoreForeignKeys = fkStatus?.foreign_keys === 1 + if (shouldRestoreForeignKeys) { migrationLog.debug(`[${tableName}] Disabling foreign key constraints for migration`) db.run("PRAGMA foreign_keys = OFF") } - db.transaction(() => { + try { + db.transaction(() => { // existing migration body - })() - - if (fkStatus && fkStatus.foreign_keys === 1) { - migrationLog.debug(`[${tableName}] Re-enabling foreign key constraints`) - db.run("PRAGMA foreign_keys = ON") + })() + } finally { + if (shouldRestoreForeignKeys) { + migrationLog.debug(`[${tableName}] Re-enabling foreign key constraints`) + db.run("PRAGMA foreign_keys = ON") + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/migration.ts` around lines 248 - 354, Summary: Re-enable PRAGMA foreign_keys in a finally block so it always runs even if migration throws. Fix: capture whether you disabled foreign keys (e.g. using fkStatus and a boolean like fkWasDisabled) before calling db.transaction(...), then wrap the transaction invocation in try { db.transaction(() => { ... }) } finally { if (fkWasDisabled) { migrationLog.debug(`[${tableName}] Re-enabling foreign key constraints`); db.run("PRAGMA foreign_keys = ON") } } so the PRAGMA ON call executes regardless of errors; keep existing migrationLog/error handling and rethrow the error from the catch inside the transaction to preserve behavior.packages/sqlite-wrapper/src/query-builder/select.ts (2)
306-358:⚠️ Potential issue | 🔴 Critical
count()andexists()still ignore accumulated JOIN clauses.Both methods rebuild SQL from
FROM ${baseTable}plus the WHERE fragment only. Joined queries therefore return the wrong cardinality, and predicates that reference joined tables can fail because the joined table never appears in the statement. These paths need to includebuildJoinClause()and prepend the join params, just likebuildSelectQuery()does.Proposed fix sketch
count(): number { if (!this.hasRegexConditions()) { + const [joinClause, joinParams] = this.buildJoinClause() const [whereClause, whereParams] = this.buildWhereClause() - const query = `SELECT COUNT(*) AS __count FROM ${quoteIdentifier(this.getTableName())}${whereClause}` + const query = `SELECT COUNT(*) AS __count FROM ${quoteIdentifier(this.getTableName())}${joinClause}${whereClause}` const result = this.getDb() .prepare(query) - .get(...whereParams) as { __count: number } | null + .get(...joinParams, ...whereParams) as { __count: number } | nullApply the same pattern in
exists().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/query-builder/select.ts` around lines 306 - 358, count() and exists() rebuild SQL using only the base FROM and WHERE fragments and thus ignore accumulated JOINs, causing wrong counts and broken predicates; update both count() and exists() to include the join clause and join params the same way buildSelectQuery() does: call buildJoinClause() (or reuse the join-building logic from buildSelectQuery()), prepend the join SQL to the FROM clause and prepend the join params to whereParams before preparing/executing the statement so JOINed tables appear in the SQL and their params are bound.
109-145:⚠️ Potential issue | 🟠 MajorDefault
SELECT *is unsafe once joins are present.With joins enabled, the fallback projection is still
*. SQLite returns duplicate column names for common fields likeid,name, or timestamps, but the row object can only keep one property per key, so one table's value overwrites the other and the merged parser becomes ambiguous. Please require an explicit projection after a join, or auto-alias joined columns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/query-builder/select.ts` around lines 109 - 145, The current buildSelectQuery method allows a default "*" projection even when joins exist, which causes duplicate column names to collide; update buildSelectQuery (and related selectedColumns handling) to detect when buildJoinClause returns a non-empty joinClause and the projection is "*" and either (a) throw a clear error requiring the caller to supply an explicit projection, or (b) auto-expand the projection by enumerating each table's columns and aliasing them (e.g., prefix/alias via quoteIdentifier(table) + "." + quoteIdentifier(col) or quoteIdentifier(table + "__" + col)) before assembling the SELECT; implement the chosen behavior inside buildSelectQuery using getTableName, buildJoinClause, selectedColumns and quoteIdentifier so joined columns are uniquely named and keep the existing ORDER/LIMIT logic intact.
🟡 Minor comments (14)
packages/sqlite-wrapper/src/lib/backup/listBackups.ts-42-42 (1)
42-42:⚠️ Potential issue | 🟡 MinorFix error interpolation in backup failure log.
Line 42 currently logs the literal
${error}instead of the real error value, which hides failure context during incident/debugging.🛠️ Proposed fix
- backupLog.error("[BACKUP] Failed to list backups | Error: ${error}", backupDir) + backupLog.error(`[BACKUP] Failed to list backups | Error: ${String(error)}`, backupDir)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/lib/backup/listBackups.ts` at line 42, In listBackups (packages/sqlite-wrapper/src/lib/backup/listBackups.ts) the log call uses a literal "${error}" instead of the thrown error; update the backupLog.error invocation to include the actual error value (and still include backupDir) by interpolating the error or passing it as a parameter to the logger so the message reads the actual error details rather than the string "${error}".packages/auth/src/index.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorRemove import-time console side effect from package entrypoint.
This log runs whenever the module is loaded and will leak noise into consumers/tests.
Suggested fix
-console.log("Hello via Bun!")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/src/index.ts` at line 1, Remove the import-time console side-effect by deleting the top-level console.log("Hello via Bun!") in the package entrypoint; if runtime debugging is needed, instead expose an explicit function (e.g., export function logHello() or init()) that performs the console.log and let callers invoke it, so loading the module no longer produces noisy logs.packages/auth/.gitignore-15-16 (1)
15-16:⚠️ Potential issue | 🟡 MinorFix glob typos so logs/reports are actually ignored.
_.logand the report pattern with underscores are too restrictive and likely won’t match intended files.Suggested fix
-_.log -report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json +*.log +report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/.gitignore` around lines 15 - 16, Replace the incorrect glob patterns in .gitignore: change "_.log" to "*.log" so all log files are ignored, and replace "report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json" with a proper numeric/glob pattern such as "report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json" (or "report.*.json" if broader matching is acceptable) so generated report files are actually excluded.packages/sqlite-wrapper/src/lib/backup/setupAutoBackup.ts-52-53 (1)
52-53:⚠️ Potential issue | 🟡 MinorFix non-interpolated interval/max-backups log message.
The log message uses double quotes instead of backticks, preventing template interpolation. The values will be logged literally as
${autoBackupOptions.intervalMs}instead of actual numeric values.Suggested fix
- "[AUTO_BACKUP] Enabled | Interval: ${autoBackupOptions.intervalMs}ms | Max backups: ${autoBackupOptions.maxBackups}", + `[AUTO_BACKUP] Enabled | Interval: ${autoBackupOptions.intervalMs}ms | Max backups: ${autoBackupOptions.maxBackups}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/lib/backup/setupAutoBackup.ts` around lines 52 - 53, The log string in setupAutoBackup is using double quotes so template expressions like ${autoBackupOptions.intervalMs} and ${autoBackupOptions.maxBackups} are not interpolated; update the logging call that currently passes "[AUTO_BACKUP] Enabled | Interval: ${autoBackupOptions.intervalMs}ms | Max backups: ${autoBackupOptions.maxBackups}", dbPath to use a template literal (backticks) so the values from autoBackupOptions are interpolated (refer to the setupAutoBackup function and the variables autoBackupOptions and dbPath when making the change).packages/sqlite-wrapper/src/lib/backup/restore.ts-41-41 (1)
41-41:⚠️ Potential issue | 🟡 MinorFix broken interpolation in restore warning log.
At Line 41,
${err}is inside double quotes, so the error value is never rendered. Change to backticks to enable template interpolation.Suggested fix
- backupLog.warn("[RESTORE] Error closing DB before restore | Error: ${err}", dbPath) + backupLog.warn(`[RESTORE] Error closing DB before restore | Error: ${String(err)}`, dbPath)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/lib/backup/restore.ts` at line 41, The warning log in restore.ts uses double quotes so the ${err} placeholder isn't interpolated; update the backupLog.warn call (the one invoking backupLog.warn("[RESTORE] Error closing DB before restore | Error: ${err}", dbPath)) to use backticks for template interpolation (i.e., change the string to a template literal) so the err value is rendered, and keep the dbPath argument as appropriate.packages/sqlite-wrapper/__test__/join-types.test.ts-114-124 (1)
114-124:⚠️ Potential issue | 🟡 MinorAmbiguous
idcolumn in joined result.Both
UserandPosthave anidfield. The comment "From User" on line 115 is misleading—when joining tables with overlapping column names, SQLite returns the last value (fromposts) unless columns are explicitly aliased. This test may pass but could give a false sense of correctness.Consider either:
- Using explicit column selection with aliases to avoid ambiguity
- Updating the comment to reflect the actual behavior
- Testing with a column unique to
User(like🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/__test__/join-types.test.ts` around lines 114 - 124, The test is asserting a non-aliased "id" which is ambiguous between User and Post; update the test (join-types.test.ts) to remove the ambiguous expect(first).toHaveProperty("id") and instead either (A) modify the query that produces `first` to explicitly select and alias columns (e.g. users.id AS user_id, posts.id AS post_id) and assert those aliases, or (B) keep the query but change assertions to rely only on unambiguous User fields (e.g. `email`) to validate User data; reference the `first` variable and the User/Post models in your change.packages/auth/package.json-1-19 (1)
1-19:⚠️ Potential issue | 🟡 MinorMissing
versionfield in package.json.The package.json lacks a
versionfield, which is required for npm packages and may cause issues with monorepo tooling.Suggested fix
{ "dependencies": { "@dockstat/logger": "workspace:*" }, "devDependencies": { "@dockstat/sqlite-wrapper": "workspace:*", "@types/bun": "latest" }, "exports": { "/client": "./src/client/index.ts", "/server": "./src/server/index.ts" }, "module": "src/index.ts", "name": "@dockstat/auth", + "version": "0.1.0", "peerDependencies": { "typescript": "^5" }, "type": "module" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/package.json` around lines 1 - 19, The package.json for the `@dockstat/auth` package is missing the top-level "version" field; add a semantically versioned string (e.g., "1.0.0" or appropriate semver) as the "version" property in the package.json to satisfy npm/monorepo tooling requirements and keep it alongside existing keys like "name", "dependencies", and "exports".packages/sqlite-wrapper/src/lib/backup/backup.ts-44-52 (1)
44-52:⚠️ Potential issue | 🟡 MinorLog message timing is misleading.
Line 46 logs
"[BACKUP] Creating backup"after theVACUUM INTOcommand has already completed. The message implies the backup is about to start, but at that point the backup file has already been created.Consider moving this log before the
db.run()call, or changing the message to reflect completion:Proposed fix
try { + backupLog.info(`[BACKUP] Creating backup`, backupPath) db.run(`VACUUM INTO '${backupPath.replace(/'/g, "''")}'`) - backupLog.info(`[BACKUP] Creating backup`, backupPath) if (autoBackupOptions) { applyRetentionPolicy(backupLog, autoBackupOptions) } backupLog.info(`[BACKUP] Backup completed successfully`, backupPath)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/lib/backup/backup.ts` around lines 44 - 52, The log timing is misleading: move the "[BACKUP] Creating backup" log to before the db.run(`VACUUM INTO ...`) call (or change its text to reflect completion) so the message accurately represents the operation; update the call to backupLog.info(`[BACKUP] Creating backup`, backupPath) to execute prior to db.run(...) (functions involved: db.run and backupLog.info) and ensure subsequent logs around applyRetentionPolicy(autoBackupOptions) and the final backupLog.info(`[BACKUP] Backup completed successfully`, backupPath) remain correct.packages/auth/package.json-9-12 (1)
9-12:⚠️ Potential issue | 🟡 MinorFix subpath export format to match Node.js ESM standard and monorepo convention.
Change
/clientand/serverto./clientand./serverto align with the standard./prefix format used throughout the rest of the monorepo and required by Node.js ESM resolution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/package.json` around lines 9 - 12, The package.json "exports" entries use "/client" and "/server" subpaths which don't follow Node.js ESM and the monorepo convention; update the exports object to use "./client" and "./server" instead (i.e., replace the "/client" and "/server" keys with "./client" and "./server") so the exports field in packages/auth/package.json correctly maps to "./src/client/index.ts" and "./src/server/index.ts".packages/sqlite-wrapper/src/index.ts-119-123 (1)
119-123:⚠️ Potential issue | 🟡 MinorLog success after the DB call returns.
These
infomessages are emitted beforenew Database(...),db.close(), and theCOMMIT/ROLLBACKstatements execute. If one of those throws, the logs still report a successful operation.Also applies to: 217-220, 462-472
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/index.ts` around lines 119 - 123, The logs (calls to this.dbLog.info) are emitted before the operations actually succeed, leading to false success messages; move each success log to after the operation completes without throwing (e.g., after this.db = new Database(path) succeeds, after db.close() returns, and after COMMIT/ROLLBACK complete) so failures are not logged as successes. Locate the early calls to this.dbLog.info around the Database constructor (this.db = new Database(path)), close logic (db.close()), and transaction commit/rollback sections and relocate or wrap them to run only after the operation finishes successfully, optionally logging errors in the catch blocks instead.packages/sqlite-wrapper/src/query-builder/where.ts-167-171 (1)
167-171:⚠️ Potential issue | 🟡 MinorNormalize fallback values the same way as
where().The non-regex branch pushes
valuedirectly intowhereParams, so booleans take a different path thanwhere(). ReusetoSqliteValue()here too.Suggested fix
- this.state.whereParams.push(value as SQLQueryBindings) + this.state.whereParams.push(this.toSqliteValue(value))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/query-builder/where.ts` around lines 167 - 171, The non-regex fallback in where.ts is pushing raw value into this.state.whereParams causing booleans (and other types) to differ from where() behavior; update the else branch in the method that builds conditions so it calls toSqliteValue(value) and pushes that normalized result into this.state.whereParams (while keeping the existing this.state.whereConditions push using quoteIdentifier(column)); reference the functions/fields to change: toSqliteValue(), where(), this.state.whereParams, this.state.whereConditions, and quoteIdentifier(column) and keep the existing whereLog.debug message.apps/dockstat/src/components/settings/accounts/sections/oauthProviders.tsx-169-191 (1)
169-191:⚠️ Potential issue | 🟡 MinorNon-functional settings button.
The Settings button (gear icon) on line 169-176 has no
onClickhandler - it renders but does nothing when clicked. Either add functionality or remove the button if editing providers isn't supported yet.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/oauthProviders.tsx` around lines 169 - 191, The Settings (gear) Button rendered in the actions cell (uses the Settings icon component) has no onClick handler and is non-functional; either remove that Button entirely or wire it to the provider-edit flow — e.g., add an onClick that calls the existing edit/open function (implement or reuse a function like openEditOAuthProvider(providerId) or showEditModal(record.providerId)) and ensure the handler is passed the record.providerId so the correct provider is edited; if editing isn't supported yet, remove the Button to avoid a dead control.apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx-96-103 (1)
96-103:⚠️ Potential issue | 🟡 MinorPotential bug:
formatDatereceives stringified null.When
value(which isexpiresAt: string | null) is null,String(value)produces the literal string"null", which is then passed toformatDate. The function only checksif (!dateString)which is falsy for empty string but not for"null". This results innew Date("null")producing an Invalid Date.🐛 Fix the null handling
{ key: "expiresAt", render: (value) => ( - <span className="text-sm text-muted-text">{formatDate(String(value))}</span> + <span className="text-sm text-muted-text">{formatDate(value as string | null)}</span> ), title: "Expires", width: "10%", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx` around lines 96 - 103, The render for the "expiresAt" column is stringifying null which yields "null" and causes formatDate to produce an Invalid Date; update the render in the expiresAt column (key "expiresAt") to check the raw value first and only call formatDate when value is non-null/defined (e.g., render: (value) => value ? formatDate(value) : a sensible fallback like "-" or empty string), or alternatively normalize to undefined instead of String(value) before passing into formatDate so formatDate's falsy check works correctly.apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx-114-122 (1)
114-122:⚠️ Potential issue | 🟡 MinorMissing delete confirmation.
The delete button immediately calls
deleteApiKeywithout confirmation. Compare this to the OAuth providers section (line 181 in oauthProviders.tsx) which shows a confirmation dialog before removal. API key deletion is destructive and should have similar protection.🛡️ Add confirmation before delete
<Button className="h-7 px-2 text-destructive hover:text-destructive" noFocusRing - onClick={() => deleteApiKey(record.id)} + onClick={() => { + if (confirm(`Are you sure you want to delete this API key?`)) { + deleteApiKey(record.id) + } + }} size="xs" variant="ghost" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx` around lines 114 - 122, The delete Button currently calls deleteApiKey(record.id) directly; change it to open a confirmation dialog/modal and only call deleteApiKey(record.id) if the user confirms (match the behavior used in the OAuth providers removal flow). Replace the onClick handler on the Button (the Trash2 button) with a function that shows a confirmation prompt (reusing your app's confirm/modal utility or the same pattern used in the oauth providers section) and on confirm invoke deleteApiKey(record.id); keep the Button props (className, noFocusRing, size, variant) unchanged so styling/behavior remains identical.
🧹 Nitpick comments (17)
knip.json (1)
1-84: Consider adding ignore patterns for common artifacts.The configuration works well as-is. Optionally, you might add
ignorepatterns to exclude common build outputs, test files, or configuration files that Knip shouldn't analyze. For example:{ "ignore": [ "**/*.test.ts", "**/*.spec.ts", "**/dist/**", "**/build/**" ], "workspaces": { ... } }This can reduce noise in Knip's reports, but it's not required if the defaults work for your project.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@knip.json` around lines 1 - 84, Add a top-level "ignore" array to the knip.json config (alongside the existing "workspaces" key) to exclude common artifacts from analysis; add patterns such as "**/*.test.ts", "**/*.spec.ts", "**/dist/**", and "**/build/**" so Knip skips test files and build outputs and reduces noise in reports while keeping the existing workspace entries intact.packages/bun-docker/src/docker.ts (1)
62-67: Consider adding JSDoc for the new public module fields.For consistency with the rest of the
DockerAPI surface, adding short doc blocks forplugins,secrets, andswarmwould improve discoverability in editors/docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bun-docker/src/docker.ts` around lines 62 - 67, Add short JSDoc one-line comments above the public readonly fields plugins, secrets, and swarm on the Docker class to match the rest of the API surface; each comment should briefly describe the module (e.g., "Access Docker plugins API", "Access Docker secrets API", "Access Docker swarm API") and follow the existing doc style used elsewhere in the file (single-line /** ... */ blocks so editors and generated docs pick them up).packages/sqlite-wrapper/src/utils/transformer.ts (1)
179-181: Avoid per-rowinfologs for normal transformations.These completion logs will fire for each row and can flood production logs on large result sets. Prefer
debugfor routine transformation telemetry.🧹 Proposed fix
- logger.info( + logger.debug( `[FROM_DB] Completed | Transformed ${transformedColumns.length} columns: ${transformedColumns.join(", ")}` ) ... - logger.info( + logger.debug( `[TO_DB] Completed | Transformed ${transformedColumns.length} columns: ${transformedColumns.join(", ")}` )Also applies to: 285-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/utils/transformer.ts` around lines 179 - 181, Change the per-row informational logs in transformer.ts to use debug-level logging so routine transformations don't flood production logs: replace logger.info calls that log transformedColumns (the log call that prints `[FROM_DB] Completed | Transformed ${transformedColumns.length} columns: ${transformedColumns.join(", ")}` and the analogous call around the code that starts at the other occurrence) with logger.debug (or guard them behind logger.isDebugEnabled() if available) so the same message is emitted only at debug verbosity; locate the calls by the transformedColumns variable and the surrounding transformation completion text in the transformer utilities (both the occurrence that logs the transformedColumns.join and the similar block at the other occurrence).packages/sqlite-wrapper/src/lib/backup/setupAutoBackup.ts (1)
37-48: Include caught error object in failure logs.Both catch blocks log only
dbPath; the actual exception is dropped, which makes incident diagnosis harder.Suggested fix
- } catch (err) { - backupLog.error("[AUTO_BACKUP] Initial backup failed", dbPath) + } catch (err) { + backupLog.error("[AUTO_BACKUP] Initial backup failed", dbPath, err) } ... - } catch (err) { - backupLog.error("[AUTO_BACKUP] Scheduled backup run failed", dbPath) + } catch (err) { + backupLog.error("[AUTO_BACKUP] Scheduled backup run failed", dbPath, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/lib/backup/setupAutoBackup.ts` around lines 37 - 48, The catch blocks in setupAutoBackup (the initial try/catch and the setInterval callback around backup and applyRetentionPolicy) currently call backupLog.error with only dbPath, dropping the caught error; update both catch handlers to capture the caught error variable (err) and pass it or its message/stack into backupLog.error alongside the existing context (e.g., "[AUTO_BACKUP] Initial backup failed" and "[AUTO_BACKUP] Scheduled backup run failed"), referencing the same symbols (backupLog.error, backup, applyRetentionPolicy, dbPath, autoBackupOptions) so logs include the actual exception details for troubleshooting.packages/auth/package.json (1)
7-7: Unpinned@types/bunversion may cause reproducibility issues.Using
"latest"for@types/bunmeans builds could break unexpectedly when new versions are released. Consider pinning to a specific version or using a caret range.Suggested fix
"devDependencies": { "@dockstat/sqlite-wrapper": "workspace:*", - "@types/bun": "latest" + "@types/bun": "^1.0.0" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/package.json` at line 7, The dependency entry for "@types/bun" in package.json is pinned to "latest", which hurts reproducibility; change it to a specific semver (e.g., "x.y.z") or a caret range ("^x.y.z") instead of "latest" so builds are deterministic—update the "@types/bun" value in package.json and run your lockfile update (npm/yarn/pnpm install) to commit the resulting lockfile changes.packages/sqlite-wrapper/README.md (1)
502-512: Minor inconsistency: missing semicolon in code example.Line 505 ends
.all()without a semicolon while other examples in the README use semicolons consistently. This is a minor cosmetic issue in documentation.Suggested fix
const results = users .join(posts, { id: "user_id" }) - .all() + .all();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/README.md` around lines 502 - 512, The README example is missing a semicolon after the .all() call; update the snippet where results is assigned (the line using users.join(posts, { id: "user_id" }).all()) to terminate the statement with a semicolon so it matches the other examples and maintains consistent punctuation for the results variable and the .join/.all() usage.packages/auth/src/server/handler/index.ts (1)
12-25:AuthHandlerclass has no methods.The class initializes
tablesandLoggerbut provides no public methods to use them. This makes the class unusable in its current state. Consider adding methods for authentication operations, or exposing table accessors if this is intended as a data-holder.Additionally, the
Loggerfield uses PascalCase which typically indicates a class/type rather than an instance field. Consider renaming tologgerfor consistency with TypeScript conventions.Suggested naming fix
export class AuthHandler { - private Logger: Logger + private logger: Logger private tables: { UsersTable: QueryBuilder<UserTable> APITokensTable: QueryBuilder<ApiTokensTable> OIDCProvidersTable: QueryBuilder<OidcProvidersTable> OIDCIdentitiesTable: QueryBuilder<UserOidcIdentitiesTable> } constructor(db: DB, logger: Logger) { - this.Logger = logger.spawn("Auth") + this.logger = logger.spawn("Auth") this.tables = createAuthTables(db, logger) } + + // TODO: Add authentication methods }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/src/server/handler/index.ts` around lines 12 - 25, AuthHandler currently only stores Logger and tables and is unusable; rename the instance field Logger to logger (update constructor and any references) to follow TS conventions, and add at least one public API such as async authenticate(credentials), async getUserById(id), or public getters for the tables (e.g., get users() returning this.tables.UsersTable) so callers can perform auth operations; ensure methods use this.logger and this.tables and are implemented within the AuthHandler class (referencing AuthHandler, createAuthTables, and the tables keys UsersTable/APITokensTable/OIDCProvidersTable/OIDCIdentitiesTable).packages/sqlite-wrapper/__test__/join-types.test.ts (1)
126-126: Remove debug comment.
// HALLOappears to be a leftover debug marker and should be removed.Suggested fix
}) - // HALLO test("where(...) should work with merged type columns", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/__test__/join-types.test.ts` at line 126, Remove the leftover debug comment "// HALLO" from the test file (join-types.test.ts) so no stray debug markers remain; locate the commented line in the test block and delete it, leaving the test code unchanged otherwise.packages/auth/src/server/db/index.ts (1)
83-85: Preserve the original error as the cause.
new Error(String(error))drops the original stack and SQLite metadata, which makes startup failures much harder to debug. Re-throwerrorwhen it's already anError, or wrap it with{ cause: error }.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth/src/server/db/index.ts` around lines 83 - 85, The catch block in auth/src/server/db/index.ts currently creates a new Error with String(error), losing the original stack and SQLite metadata; instead, preserve the original error by either re-throwing the caught error directly when it's already an Error (throw error) or, if you must wrap it, create a new Error that includes the original as the cause (new Error("Could not create Auth Tables", { cause: error })), and update the logger.error call to include the error object so the stack/metadata are logged (logger.error("Could not create Auth Tables!", error)).apps/dockstat/src/components/settings/accounts/index.tsx (2)
2-2: Unused import:Settingsicon.The
Settingsicon is imported but not used in this component. It appears to be used only in the childOAuthProvidersSectioncomponent which has its own import.🧹 Remove unused import
-import { Key, Settings, Shield } from "lucide-react" +import { Key, Shield } from "lucide-react"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/index.tsx` at line 2, Remove the unused Settings import from the import statement that currently reads "import { Key, Settings, Shield } from 'lucide-react'": edit the import to only include the icons actually used in this file (e.g., Key and Shield) since the Settings icon is not referenced here (it is imported and used inside OAuthProvidersSection), so delete Settings from the named imports to eliminate the unused import.
46-139: Consider extracting the repeated section header pattern.Each of the three sections (Users, API Keys, OAuth Providers) follows the same nested Card header structure. This could be extracted into a reusable
SectionCardcomponent to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/index.tsx` around lines 46 - 139, The three repeated header blocks should be extracted into a reusable SectionCard component: create a SectionCard(props) that accepts title: string, icon: JSX.Element, children: ReactNode (or a prop for CardBody content) and any Card props (variant/size/className), and renders the outer Card, the nested header Card with icon + title, then a CardBody wrapper for children; then replace the three manual header+CardBody blocks that wrap UsersSection, ApiKeysSection and OAuthProvidersSection with SectionCard usage (pass Shield/Key/Settings as icon, "User Management"/"API Keys"/"OAuth Providers" as title, and move banUser/createUser/…/oauthProviders props into the children components).apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx (2)
7-12: Prop types don't reflect async nature of callbacks.The callback props are typed as returning
void, but the actual implementations fromuseAccountsare async functions returningPromise<void>. While this works due to how TypeScript handles void returns, updating the types toPromise<void>would be more accurate and allow consumers to await these operations if needed.📝 More accurate typing
interface ApiKeysSectionProps { apiKeys: ApiKey[] - createApiKey: (data: { name: string; referenceId: string }) => void - deleteApiKey: (id: string) => void - toggleApiKey: (id: string, enabled: boolean) => void + createApiKey: (data: { name: string; referenceId: string }) => Promise<void> + deleteApiKey: (id: string) => Promise<void> + toggleApiKey: (id: string, enabled: boolean) => Promise<void> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx` around lines 7 - 12, The prop types in ApiKeysSectionProps misrepresent the async callbacks: update the signatures for createApiKey, deleteApiKey, and toggleApiKey to return Promise<void> instead of void so they match the async implementations from useAccounts; keep the same parameter shapes (createApiKey: (data: { name: string; referenceId: string }) => Promise<void>, deleteApiKey: (id: string) => Promise<void>, toggleApiKey: (id: string, enabled: boolean) => Promise<void>), and then adjust any local callers to await these promises where needed.
32-34: Missing user feedback for clipboard copy.The
copyToClipboardfunction silently copies to clipboard without any visual feedback. Consider adding a toast notification or temporary visual indicator so users know the copy succeeded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx` around lines 32 - 34, The copyToClipboard function in apps/dockstat/src/components/settings/accounts/sections/apiKeys.tsx currently writes silently to the clipboard; update copyToClipboard to handle the promise from navigator.clipboard.writeText(text) and provide user feedback by showing a success or error toast (or a temporary visual indicator state) when the copy resolves or rejects. Locate the copyToClipboard function and any UI element that triggers it, call the toast helper (or set a transient "copied" state) on success and show an error toast on failure, and ensure the UI clears the transient indicator after a short timeout.packages/sqlite-wrapper/src/lib/backup/applyRetentionPolicy.ts (1)
32-37: Semantic mismatch:backupDirpassed asrequestIdparameter.The Logger's
debug/errormethods have signature(msg: string, requestId?: string). PassingbackupDiras the second argument works but is semantically incorrect - it's a directory path, not a request ID. The logs will show the backup directory in the request ID field.Consider embedding
backupDirin the message string instead for clarity, or verify this is intentional for log correlation purposes.💡 Suggested alternative
- backupLog.debug(`[RETENTION] Removing old backup: ${file.name}`, backupDir) + backupLog.debug(`[RETENTION] Removing old backup: ${file.name} | Directory: ${backupDir}`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sqlite-wrapper/src/lib/backup/applyRetentionPolicy.ts` around lines 32 - 37, The log calls in applyRetentionPolicy are passing backupDir as the second parameter to backupLog.debug/info/error (which is the requestId parameter) causing a semantic mismatch; update calls that currently pass backupDir (e.g., backupLog.debug(`[RETENTION] Removing old backup: ${file.name}`, backupDir), backupLog.info(..., backupDir) and backupLog.error(`[RETENTION] Failed to apply retention policy | Error: ${error}`, backupDir)) to instead include backupDir inside the message string (or omit the second argument) so the requestId field is not misused—search for usages of backupLog.debug/backupLog.info/backupLog.error in applyRetentionPolicy and change them accordingly.apps/dockstat/src/components/settings/accounts/sections/oauthProviders.tsx (2)
438-442: Hardcoded localhost URL in setup instructions.The redirect URI shows
http://localhost:3030/auth/callback/{provider}which is only valid during local development. In production, this would need to be the actual domain. Consider deriving this from the current window location or an environment variable.💡 Use dynamic URL
<li> Set the redirect URI to:{" "} <code className="bg-muted/10 px-1 rounded"> - http://localhost:3030/auth/callback/{selectedProvider.id} + {window.location.origin}/auth/callback/{selectedProvider.id} </code> </li>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/oauthProviders.tsx` around lines 438 - 442, The hardcoded redirect URI should be built dynamically instead of using http://localhost:3030; update the JSX that renders the redirect URI (where it uses selectedProvider.id) to compute the base URL from a config/env value (e.g., a VITE/REACT_APP style env var) with a fallback to window.location.origin, and render `${baseOrigin}/auth/callback/${selectedProvider.id}` so the displayed URI matches production or local environments; ensure the symbol selectedProvider.id is used unchanged and prefer an environment var like REACT_APP_BASE_URL or VITE_BASE_URL for deployments.
52-58: Potential runtime error:OAUTH_SCOPESaccess for custom-oidc provider.When
selectedProvider.type === "oauth"but the provider is not inOAUTH_SCOPES(though current configs only have oauth providers that exist in OAUTH_SCOPES), the accessOAUTH_SCOPES[selectedProvider.id as keyof typeof OAUTH_SCOPES]would return undefined. This is currently safe given the PROVIDER_CONFIGS, but adding a fallback would make it more robust.🛡️ Add defensive fallback
const scopes = selectedProvider.type === "oidc" ? selectedScopes.length > 0 ? selectedScopes : ["openid", "profile", "email"] - : Array.from(OAUTH_SCOPES[selectedProvider.id as keyof typeof OAUTH_SCOPES]) + : Array.from(OAUTH_SCOPES[selectedProvider.id as keyof typeof OAUTH_SCOPES] ?? [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/oauthProviders.tsx` around lines 52 - 58, The current scopes computation can throw if OAUTH_SCOPES[selectedProvider.id] is undefined for custom oauth providers; update the ternary branch for the oauth case to safely fall back to an empty array (or sensible default) before calling Array.from. Concretely, in the scopes expression use a guarded lookup like const providerScopes = OAUTH_SCOPES[selectedProvider.id as keyof typeof OAUTH_SCOPES] ?? []; then use Array.from(providerScopes) (or providerScopes directly) so selectedProvider, selectedScopes and OAUTH_SCOPES are referenced but undefined entries no longer cause a runtime error.apps/dockstat/src/components/settings/accounts/sections/useAccounts.tsx (1)
92-97: Unsafe type assertion from localStorage.The value from
localStorage.getItem()is cast directly toAdminModalOptionwithout validation. If localStorage contains an unexpected value (corrupted data, manual tampering), this could lead to unexpected behavior.🛡️ Add validation before casting
useEffect(() => { const savedChoice = localStorage.getItem("dockstat-admin-modal-choice") - if (savedChoice) { - setAdminModalChoice(savedChoice as AdminModalOption) + if (savedChoice && ["yes", "no", "ask-later"].includes(savedChoice)) { + setAdminModalChoice(savedChoice as AdminModalOption) } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dockstat/src/components/settings/accounts/sections/useAccounts.tsx` around lines 92 - 97, The code reads localStorage.getItem("dockstat-admin-modal-choice") and casts it directly to AdminModalOption in the useEffect; instead validate the raw string against the allowed AdminModalOption values before calling setAdminModalChoice to avoid unsafe assertion. Implement or use a guard (e.g., isAdminModalOption or an array of permitted values) to check the savedChoice is one of the enum/string union members, and only then call setAdminModalChoice(savedChoice); otherwise ignore it or set a safe default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b42d9aa-6215-49b0-996d-8e26c614b0cb
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (53)
.zed/settings.jsonapps/api/package.jsonapps/api/src/auth/db.tsapps/api/src/auth/index.tsapps/api/src/database/index.tsapps/api/src/elysia-plugins.tsapps/api/src/index.tsapps/api/tsconfig.jsonapps/docknode/src/_treaty.tsapps/dockstat/src/components/settings/accounts/index.tsxapps/dockstat/src/components/settings/accounts/sections/adminModal.tsxapps/dockstat/src/components/settings/accounts/sections/apiKeys.tsxapps/dockstat/src/components/settings/accounts/sections/oauthProviders.tsxapps/dockstat/src/components/settings/accounts/sections/useAccounts.tsxapps/dockstat/src/components/settings/accounts/sections/users.tsxapps/dockstat/src/hooks/mutations/accounts.tsapps/dockstat/src/hooks/mutations/index.tsapps/dockstat/src/pages/settings.tsxapps/dockstat/tsconfig.jsonknip.jsonpackages/auth/.gitignorepackages/auth/README.mdpackages/auth/package.jsonpackages/auth/src/client/index.tspackages/auth/src/index.tspackages/auth/src/server/db/index.tspackages/auth/src/server/db/types.tspackages/auth/src/server/db/utils.tspackages/auth/src/server/handler/index.tspackages/auth/src/server/index.tspackages/auth/tsconfig.jsonpackages/bun-docker/src/docker.tspackages/sqlite-wrapper/README.mdpackages/sqlite-wrapper/__test__/join-types.test.tspackages/sqlite-wrapper/__test__/transformer.test.tspackages/sqlite-wrapper/package.jsonpackages/sqlite-wrapper/src/index.tspackages/sqlite-wrapper/src/lib/backup/applyRetentionPolicy.tspackages/sqlite-wrapper/src/lib/backup/backup.tspackages/sqlite-wrapper/src/lib/backup/listBackups.tspackages/sqlite-wrapper/src/lib/backup/restore.tspackages/sqlite-wrapper/src/lib/backup/setupAutoBackup.tspackages/sqlite-wrapper/src/migration.tspackages/sqlite-wrapper/src/query-builder/base.tspackages/sqlite-wrapper/src/query-builder/delete.tspackages/sqlite-wrapper/src/query-builder/index.tspackages/sqlite-wrapper/src/query-builder/insert.tspackages/sqlite-wrapper/src/query-builder/join.tspackages/sqlite-wrapper/src/query-builder/select.tspackages/sqlite-wrapper/src/query-builder/update.tspackages/sqlite-wrapper/src/query-builder/where.tspackages/sqlite-wrapper/src/types.tspackages/sqlite-wrapper/src/utils/transformer.ts
| @@ -0,0 +1,17 @@ | |||
| import type DB from "@dockstat/sqlite-wrapper" | |||
| import { column } from "@dockstat/sqlite-wrapper" | |||
| import { DockStatDB } from "../database" | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Cycle check (direct reciprocal imports):"
rg -n --type=ts -C1 'from "../database"' apps/api/src/auth/db.ts
rg -n --type=ts -C1 'from "../auth/db"' apps/api/src/database/index.tsRepository: Its4Nik/DockStat
Length of output: 408
🏁 Script executed:
cat -n apps/api/src/auth/db.tsRepository: Its4Nik/DockStat
Length of output: 637
🏁 Script executed:
grep -n "DockStatDB" apps/api/src/auth/db.tsRepository: Its4Nik/DockStat
Length of output: 102
Remove unused import that creates circular module dependency.
DockStatDB is imported on line 3 but never used. This import creates a circular dependency: apps/api/src/auth/db.ts imports from ../database, while apps/api/src/database/index.ts imports getAuthTable from apps/api/src/auth/db.ts. Circular dependencies can cause partially initialized modules at runtime.
Suggested fix
-import { DockStatDB } from "../database"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { DockStatDB } from "../database" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/auth/db.ts` at line 3, Remove the unused import DockStatDB from
the top of the module to eliminate the circular dependency; specifically delete
the import statement that references DockStatDB (it is not used anywhere in this
file) and keep getAuthTable and related exports intact so other modules can
import them without causing partial initialization; run tests or a type-check to
confirm no remaining references and, if you need the type only, convert it to a
type-only import (e.g., using "import type") instead of a runtime import.
| pass: column.text(), | ||
| token: column.text(), |
There was a problem hiding this comment.
Do not persist raw passwords or API tokens.
pass and token columns represent plaintext secret storage; compromise of the DB immediately compromises user credentials and API access.
Suggested schema direction
- pass: column.text(),
- token: column.text(),
+ passwordHash: column.text(),
+ tokenHash: column.text(),Also update writers/verifiers to hash before insert and compare hash on authentication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/auth/db.ts` around lines 12 - 13, The schema currently persists
plaintext secrets in the columns named "pass" and "token"; change these to store
hashed values (rename to e.g., "pass_hash" and "token_hash" or document they
contain hashes) and keep their type as text, then update any insert/update logic
in the auth writers to hash secrets before saving and update
authentication/verifier code to compare using a secure hash verify call (use a
modern library such as bcrypt or argon2 for pass hashing and a suitable keyed
HMAC or one-way hash for API tokens). Ensure functions/methods that write to the
DB (writers) call the hashing routine before insert, and functions that
authenticate (verifiers) use the corresponding verify/compare routine (e.g.,
bcrypt.compare/argon2.verify) against "pass_hash"/"token_hash"; also plan a
migration/rotation strategy for existing plaintext entries.
| createApiKey(name: string) { | ||
| this.logger.info(`Creating API Key ${name}`) | ||
| const token = `DAPI-${Bun.randomUUIDv7("base64")}` | ||
| return this.table.insertAndGet({ name, token, type: "api-key" }) | ||
| } | ||
|
|
||
| // biome-ignore lint/suspicious/noAssignInExpressions: From the better auth examples | ||
| const getSchema = async () => (_schema ??= auth.api.generateOpenAPISchema()) | ||
| createUser(name: string, password: string) { | ||
| this.logger.info(`Creating user ${name}`) | ||
| return this.table.insertAndGet({ name, pass: password, type: "user" }) |
There was a problem hiding this comment.
Never persist raw API keys or passwords.
createApiKey() stores the bearer token directly, and createUser() stores the password directly. A read-only DB leak becomes immediate credential compromise. Store a password hash plus a token digest instead, and return the raw API key only once at creation time. This also diverges from packages/auth/src/server/db/types.ts, which already models passwordHash and tokenHash.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/auth/index.ts` around lines 9 - 17, createApiKey and createUser
are persisting raw credentials; change them to hash secrets before storing and
return the raw API key only at creation: in createApiKey generate the raw token
(e.g. DAPI-...), compute a secure token digest (tokenHash) using a strong
KDF/HMAC, store { name, tokenHash, type: "api-key" } via
this.table.insertAndGet, and return the raw token to the caller only once; in
createUser compute a passwordHash (using bcrypt/argon2/scrypt) from the provided
password and store { name, passwordHash, type: "user" } instead of pass,
updating any field names to match packages/auth/src/server/db/types.ts and
keeping table.insertAndGet usage. Ensure error handling and consistent field
names (passwordHash/tokenHash) so raw secrets are never persisted.
| "paths": { | ||
| "@dockstat/api": ["./src/*"] | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For TypeScript paths, what is the correct configuration when you want both @dockstat/api(root module) and@dockstat/api/* (subpath modules) to resolve in the same project?
💡 Result:
The correct TypeScript paths configuration to resolve both @dockstat/api (root module, typically mapping to an index file like src/api/index.ts) and @dockstat/api/* (subpaths like @dockstat/api/utils) in the same project is to explicitly define both mappings in tsconfig.json. Assume your API code is in a src/api/ directory with an index.ts file re-exporting the main API, and submodules like src/api/utils.ts. { "compilerOptions": { "baseUrl": ".", "paths": { "@dockstat/api": ["src/api/index.ts"], "@dockstat/api/": ["src/api/"] } } } ### Key Points: - Both entries are required: TypeScript's path mapping algorithm uses longest prefix matching. - import { something } from "@dockstat/api" → exact match to "@dockstat/api" → resolves to src/api/index.ts - import { util } from "@dockstat/api/utils" → longest match "@dockstat/api/*" (wildcard captures "utils") → resolves to src/api/utils - Without the exact "@dockstat/api" entry, root imports fail because wildcards don't automatically resolve bare package names to index.ts. - baseUrl is mandatory for paths to work (set to "." for tsconfig root, or "src" if paths are relative to src). - Paths don't affect emitted JS (use bundlers like webpack/Vite or tsconfig-paths for runtime). ### Usage Examples: import { ApiClass } from "@dockstat/api"; // → src/api/index.ts import { helperFn } from "@dockstat/api/utils"; // → src/api/utils.ts ### Runtime Support: - Development: Use ts-node -r tsconfig-paths/register - Build tools: Configure webpack (tsconfig-paths-webpack-plugin), Vite (path: { '@dockstat/api': ... }), etc. - Node.js: Use package.json "imports" for native subpath support (TS 5.0+ with moduleResolution: "node16"). This pattern is confirmed across official docs, GitHub issues (#5039, #26859), and community examples.
Citations:
- 1: https://www.typescriptlang.org/tsconfig/paths.html
- 2: Path mappings based module resolution microsoft/TypeScript#5039
- 3: TS Paths not working with index.ts when using a wildcard - more explained inside microsoft/TypeScript#26859
- 4: https://stackoverflow.com/questions/50679031/why-are-these-tsconfig-paths-not-working
- 5: https://stackoverflow.com/questions/43281741/how-can-i-use-paths-in-tsconfig-json
- 6: https://microsoft-typescript.mintlify.app/compiler/module-resolution
- 7: https://www.cosmiclearn.com/typescript/module_resolution.php
🏁 Script executed:
# Verify the file and lines in question
if [ -f "apps/api/tsconfig.json" ]; then
echo "=== File exists ==="
sed -n '60,70p' apps/api/tsconfig.json
fiRepository: Its4Nik/DockStat
Length of output: 1253
🏁 Script executed:
# Check if there's an entry point file
ls -la apps/api/src/ 2>/dev/null | head -20Repository: Its4Nik/DockStat
Length of output: 1058
🏁 Script executed:
# Check how `@dockstat/api` is being imported in the codebase
rg "@dockstat/api" --type ts --type tsx -A 1 -B 1 | head -50Repository: Its4Nik/DockStat
Length of output: 87
🏁 Script executed:
# Also check package.json to see if there's any related configuration
if [ -f "apps/api/package.json" ]; then
cat apps/api/package.json
fiRepository: Its4Nik/DockStat
Length of output: 1213
🏁 Script executed:
# Search for imports of `@dockstat/api` to see how it's used
rg "@dockstat/api" --type ts -A 1 -B 1 | head -80Repository: Its4Nik/DockStat
Length of output: 218
🏁 Script executed:
# Also check the baseUrl setting in the tsconfig
grep -A 5 -B 5 "baseUrl" apps/api/tsconfig.jsonRepository: Its4Nik/DockStat
Length of output: 947
Split the path alias into exact and wildcard entries to properly resolve root and subpath imports.
The current configuration "@dockstat/api": ["./src/*"] resolves root imports only by relying on implicit index file discovery. To correctly handle both @dockstat/api (root module) and @dockstat/api/* (subpaths), split into explicit mappings:
Suggested fix
"paths": {
- "@dockstat/api": ["./src/*"]
+ "@dockstat/api": ["./src/index.ts"],
+ "@dockstat/api/*": ["./src/*"]
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "paths": { | |
| "@dockstat/api": ["./src/*"] | |
| }, | |
| "paths": { | |
| "@dockstat/api": ["./src/index.ts"], | |
| "@dockstat/api/*": ["./src/*"] | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/tsconfig.json` around lines 64 - 66, The path alias "@dockstat/api"
currently maps only to "./src/*" and therefore doesn't explicitly resolve the
package root import; update the tsconfig "paths" so you have two entries: an
exact key "@dockstat/api" mapped to the package root (e.g., "./src" or the root
entry file such as "./src/index.ts") and a wildcard key "@dockstat/api/*" mapped
to "./src/*" so both import "@dockstat/api" and "@dockstat/api/..." resolve
correctly.
| const handleCreate = () => { | ||
| if (!newUserName || !newUserEmail || !newUserPassword) return | ||
| createUser({ email: newUserEmail, name: newUserName, password: newUserPassword }) | ||
| setNewUserName("") | ||
| setNewUserEmail("") | ||
| setNewUserPassword("") | ||
|
|
||
| setShowCreateForm(false) | ||
| } | ||
|
|
||
| const handleBan = () => { | ||
| if (!selectedUser || !banReason) return | ||
| banUser(selectedUser.id, banReason) | ||
| setShowBanModal(false) | ||
| setSelectedUser(null) | ||
| setBanReason("") |
There was a problem hiding this comment.
Reset the create/ban UI only after the mutation succeeds.
Both handlers clear local state immediately after calling createUser() / banUser(). If those callbacks fail asynchronously, the modal closes and the entered values are lost, leaving the admin with no retry path. Move the reset into an explicit success callback/promise resolution instead of doing it optimistically.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dockstat/src/components/settings/accounts/sections/users.tsx` around
lines 45 - 60, handleCreate and handleBan currently clear form and close modals
immediately after calling createUser() / banUser(); change them to only reset
local state and close the UI after the mutation succeeds by awaiting the promise
or using the mutation's onSuccess callback. Specifically, in handleCreate use
the promise returned by createUser (or the createUser hook's onSuccess) to call
setNewUserName(""), setNewUserEmail(""), setNewUserPassword("") and
setShowCreateForm(false) only on success, and in handleBan call
banUser(...).then/onSuccess to call setShowBanModal(false),
setSelectedUser(null) and setBanReason("") only on success, and keep early
returns for invalid input unchanged. Ensure errors are still surfaced (do not
swallow promise rejections).
|
|
||
| export const getUserByID = (id: string, table: QueryBuilder<UserTable>) => | ||
| table.select(["*"]).where({ id: id }).first() | ||
| export const getUserByToken = (token: string, db: DB) => {} |
There was a problem hiding this comment.
getUserByToken has an empty implementation.
This function accepts token and db parameters but has no implementation body. It will return undefined, which will likely cause issues when callers expect user data. Either implement the function or mark it as a stub with a TODO comment and appropriate return type.
Suggested stub with explicit TODO
-export const getUserByToken = (token: string, db: DB) => {}
+// TODO: Implement token-based user lookup
+export const getUserByToken = (token: string, db: DB): null => {
+ throw new Error("getUserByToken not implemented")
+}Would you like me to help implement this function or open an issue to track it?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getUserByToken = (token: string, db: DB) => {} | |
| // TODO: Implement token-based user lookup | |
| export const getUserByToken = (token: string, db: DB): never => { | |
| throw new Error("getUserByToken not implemented") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/auth/src/server/db/utils.ts` at line 6, getUserByToken currently has
an empty body; implement it to return the user associated with the given token
(or explicitly return null) instead of undefined. Locate the
getUserByToken(token: string, db: DB) function and either: 1) implement the
lookup by calling your DB's session/token methods (e.g.,
db.findSessionByToken(token) then db.getUserById(session.userId) or
db.findUserByToken(token)) and return the User or null (make the function
async/return Promise<User | null> if DB calls are async), or 2) if the
repository API is not available yet, add a clear TODO and return null with the
proper return type (or throw a NotImplementedError) so callers don't receive
undefined; ensure the function signature and JSDoc/type annotations reflect the
chosen return type.
| if (dbPath === ":memory:") { | ||
| backupLog.warn("Auto-backup is not available for in-memory databases") | ||
| backupLog.warn("[AUTO_BACKUP] Not available for in-memory databases", dbPath) | ||
| } |
There was a problem hiding this comment.
Short-circuit auto-backup for in-memory databases.
After warning, execution continues and still configures backup flow. For :memory:, return early with timer: null to avoid repeated failing work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sqlite-wrapper/src/lib/backup/setupAutoBackup.ts` around lines 13 -
15, The function currently logs and continues when dbPath === ":memory:",
causing the backup flow to still be configured; modify setupAutoBackup so after
calling backupLog.warn("[AUTO_BACKUP] Not available for in-memory databases",
dbPath) it immediately returns the expected shape with timer: null (and any
other required return fields) to short-circuit setup and avoid scheduling or
running backup work for in-memory databases; locate the check using dbPath and
backupLog.warn in setupAutoBackup and add the early return there.
| if (typeof condition === "string") { | ||
| // Raw expression condition | ||
| onClause = ` ON ${condition}` | ||
| } else { |
There was a problem hiding this comment.
Handle CROSS JOIN without an ON clause.
crossJoin() records condition: "", and this branch renders every string condition as ON ${condition}. The generated SQL becomes ... CROSS JOIN ... ON , which is invalid SQLite syntax. Special-case type === "CROSS" here, or reject empty string conditions for every non-cross join.
Proposed fix
- if (typeof condition === "string") {
- // Raw expression condition
- onClause = ` ON ${condition}`
+ if (type === "CROSS") {
+ onClause = ""
+ } else if (typeof condition === "string") {
+ if (!condition.trim()) {
+ throw new Error("JOIN condition cannot be empty")
+ }
+ onClause = ` ON ${condition}`
} else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sqlite-wrapper/src/query-builder/base.ts` around lines 183 - 186,
The code currently treats any string condition as an ON clause (setting onClause
= ` ON ${condition}`), which produces invalid SQL for crossJoin() which records
condition as "" — update the branch in the query builder that sets onClause so
that when condition is a string and either non-empty or the join type is not
"CROSS" you render ` ON ${condition}`, but if type === "CROSS" and condition ===
"" omit the ON clause entirely; alternatively validate/throw for empty string
conditions on non-CROSS joins. Refer to the condition variable, onClause
assignment, and the join type check (type === "CROSS") and the crossJoin()
call-site to implement this special-case.
| join<JT extends Record<string, unknown>>( | ||
| table: QueryBuilder<JT, JT>, | ||
| condition: JoinCondition, | ||
| alias?: string | ||
| ): QueryBuilder<T, ResultType & JT> { | ||
| this.selectBuilder.join<JT>(table, condition, alias) | ||
| return this as unknown as QueryBuilder<T, ResultType & JT> | ||
| } | ||
|
|
||
| /** | ||
| * Add an INNER JOIN clause. | ||
| * | ||
| * @template JT - The joined table type | ||
| * @returns A new QueryBuilder with the merged result type (ResultType & JT) | ||
| */ | ||
| innerJoin<JT extends Record<string, unknown>>( | ||
| table: QueryBuilder<JT, JT>, | ||
| condition: JoinCondition, | ||
| alias?: string | ||
| ): QueryBuilder<T, ResultType & JT> { | ||
| this.selectBuilder.innerJoin<JT>(table, condition, alias) | ||
| return this as unknown as QueryBuilder<T, ResultType & JT> | ||
| } | ||
|
|
||
| /** | ||
| * Add a LEFT JOIN clause. | ||
| * | ||
| * @template JT - The joined table type | ||
| * @returns A new QueryBuilder with the merged result type (ResultType & JT) | ||
| */ | ||
| leftJoin<JT extends Record<string, unknown>>( | ||
| table: QueryBuilder<JT, JT>, | ||
| condition: JoinCondition, | ||
| alias?: string | ||
| ): QueryBuilder<T, ResultType & JT> { | ||
| this.selectBuilder.leftJoin<JT>(table, condition, alias) | ||
| return this as unknown as QueryBuilder<T, ResultType & JT> | ||
| } | ||
|
|
||
| /** | ||
| * Add a RIGHT JOIN clause. | ||
| * | ||
| * @template JT - The joined table type | ||
| * @returns A new QueryBuilder with the merged result type (ResultType & JT) | ||
| */ | ||
| rightJoin<JT extends Record<string, unknown>>( | ||
| table: QueryBuilder<JT, JT>, | ||
| condition: JoinCondition, | ||
| alias?: string | ||
| ): QueryBuilder<T, ResultType & JT> { | ||
| this.selectBuilder.rightJoin<JT>(table, condition, alias) | ||
| return this as unknown as QueryBuilder<T, ResultType & JT> | ||
| } | ||
|
|
||
| /** | ||
| * Add a FULL JOIN clause. | ||
| * | ||
| * @template JT - The joined table type | ||
| * @returns A new QueryBuilder with the merged result type (ResultType & JT) | ||
| */ | ||
| fullJoin<JT extends Record<string, unknown>>( | ||
| table: QueryBuilder<JT, JT>, | ||
| condition: JoinCondition, | ||
| alias?: string | ||
| ): QueryBuilder<T, ResultType & JT> { | ||
| this.selectBuilder.fullJoin<JT>(table, condition, alias) | ||
| return this as unknown as QueryBuilder<T, ResultType & JT> | ||
| } | ||
|
|
||
| /** | ||
| * Add a CROSS JOIN clause. | ||
| * | ||
| * @template JT - The joined table type | ||
| * @returns A new QueryBuilder with the merged result type (ResultType & JT) | ||
| */ | ||
| crossJoin<JT extends Record<string, unknown>>( | ||
| table: QueryBuilder<JT, JT>, | ||
| alias?: string | ||
| ): QueryBuilder<T, ResultType & JT> { | ||
| this.selectBuilder.crossJoin<JT>(table, alias) | ||
| return this as unknown as QueryBuilder<T, ResultType & JT> | ||
| } |
There was a problem hiding this comment.
These JOIN methods retype the same mutable instance instead of returning a new typed builder.
Each method mutates this.selectBuilder and then casts this to QueryBuilder<T, ResultType & JT>. That breaks the advertised type-safety: any older reference to the same builder keeps the old static type but now runs the joined query at runtime. Either clone the builder before adding the join, or keep the original generic and avoid claiming that a fresh merged-type builder was returned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sqlite-wrapper/src/query-builder/index.ts` around lines 191 - 272,
The join methods (join, innerJoin, leftJoin, rightJoin, fullJoin, crossJoin)
currently mutate this.selectBuilder and then cast and return this as
QueryBuilder<T, ResultType & JT>, which lies about the instance's type; fix by
either (A) cloning the builder state before mutating: create a new QueryBuilder
instance (copying current selectBuilder, where clauses, params, etc.), call the
corresponding selectBuilder.*Join on the clone, and return that new
QueryBuilder<T, ResultType & JT>, or (B) preserve correctness by not changing
the generic result type and instead return this (remove the cast) so the
signature reflects the actual mutated instance; choose one approach and apply it
consistently to all listed methods (join, innerJoin, leftJoin, rightJoin,
fullJoin, crossJoin).
| } catch { | ||
| // Keep original value if JSON parsing fails | ||
| logger.warn(`Failed to parse JSON column: ${columnKey}`) | ||
| logger.warn(`[FROM_DB] Failed to parse JSON column: ${value}`, columnKey) |
There was a problem hiding this comment.
Do not log raw DB values in warning paths.
Lines 67 and 150 include raw column values in warning logs. This can expose sensitive data/PII to log sinks.
🔒 Proposed fix
- logger.warn(`[FROM_DB] Failed to parse JSON column: ${value}`, columnKey)
+ logger.warn(`[FROM_DB] Failed to parse JSON column`, columnKey)
...
- logger.warn(`[FROM_DB] Failed to parse DATE column Value: ${value}`, columnKey)
+ logger.warn(`[FROM_DB] Failed to parse DATE column`, columnKey)Also applies to: 150-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sqlite-wrapper/src/utils/transformer.ts` at line 67, The warning
logs in transformer.ts currently include raw DB values (the logger.warn call
that builds "[FROM_DB] Failed to parse JSON column: ${value}", columnKey and the
equivalent at the second occurrence), which can leak sensitive data; change both
logger.warn usages to avoid logging the raw value — log the columnKey and error
details (e.g. error.message) and, if useful, a non-sensitive artifact such as
the value length or a fixed "<REDACTED>" placeholder instead of the actual
value; update the two logger.warn calls that reference "[FROM_DB] Failed to
parse JSON column" so they never interpolate the DB value directly.
1 participant
Summary by CodeRabbit
New Features
Refactor
Chores