refactor: short id branded type

[nicotsx]

Summary by CodeRabbit

• Refactor

  • Strengthened type validation for resource identifiers to improve data integrity across volumes, repositories, and backup schedules.

• Chores

  • Updated TanStack React Router and related dependencies to the latest versions.
  • Enhanced build configuration with improved import protection.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR introduces a branded TypeScript type system for short identifiers (ShortId) and migrates volume, repository, and backup schedule identification from numeric/string types to strongly-typed ShortId values. Changes span database schema annotations, service method signatures, controller parameter normalization, background jobs, and test fixtures.

Changes

Cohort / File(s)	Summary
Branded Type System app/server/utils/branded.ts, app/server/utils/id.ts	Introduces Branded type utility, ShortId branded type, asShortId() converter, and isShortId() validator. Updates generateShortId() to return ShortId type.
Database Schema app/server/db/schema.ts	Adds ShortId type annotation to short_id columns in volumesTable, repositoriesTable, and backupSchedulesTable using $type<ShortId>().
Volume Service & Controller app/server/modules/volumes/volume.service.ts, app/server/modules/volumes/volume.controller.ts	Updates service method signatures (findVolume, deleteVolume, mountVolume, etc.) from string | number to ShortId. Controller normalizes route parameters with asShortId().
Repository Service & Controller app/server/modules/repositories/repositories.service.ts, app/server/modules/repositories/repositories.controller.ts	Replaces string parameters with ShortId across all exported methods (getRepository, listSnapshots, checkHealth, etc.). Controller applies asShortId() to incoming shortId and backupId parameters.
Backup Service & Controller app/server/modules/backups/backups.service.ts, app/server/modules/backups/backups.controller.ts	Updates getScheduleByShortId to accept ShortId. Changes reorderSchedules signature to ShortId[]. Controller wraps all shortId extractions and body mappings with asShortId().
Background Jobs app/server/jobs/auto-remount.ts, app/server/jobs/healthchecks.ts, app/server/jobs/repository-healthchecks.ts, app/server/modules/lifecycle/startup.ts	Replaces volume.id and repository.id references with volume.shortId and repository.shortId in service invocations.
Test Helpers & Tests app/test/helpers/volume.ts, app/test/helpers/repository.ts, app/test/helpers/backup.ts, app/server/modules/volumes/__tests__/volumes.service.test.ts	Updates test fixture generators to use generateShortId() instead of faker utilities. Test assertions updated to pass ShortId values to service methods.
Configuration & Dependencies package.json, vite.config.ts	Updates TanStack router, router-ssr-query, and react-start packages to ^1.161.3. Adds importProtection configuration with "error" behavior to tanstackStart vite plugin.

Possibly related PRs

• refactor: always use short id in api calls #545: Performs identical refactor switching identifiers from numeric/id to branded shortId across controllers, services, and database schema.
• refactor: tag snapshots with schedule short_id instead of db id #248: Modifies backup schedule short_id handling in the same schema and backup service logic paths.
• fix(repositories): append short id when using the default path for local repos #536: Updates repositories.service.ts with shortId handling for local repository path generation.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor: short id branded type' clearly and directly summarizes the main change: introducing a branded type system for short IDs across the codebase.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[nicotsx]

• refactor: react-doctor #553
• refactor: short id branded type #552 👈 (View in Graphite)
• feat: repository used space #551
• feat: restore snapshot as tar #550
• refactor: always use short id in api calls #545
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[dosubot]

Documentation Updates

1 document(s) were updated by changes in this PR:

Zerobyte Startup Orchestration

View Changes

@@ -66,6 +66,17 @@
 - **Notifications**: `notificationDestinationsTable`
 - **Metadata**: `appMetadataTable` for migration checkpoints and system state
 
+### Resource Identifier Type Safety
+Volumes, repositories, and backup schedules use a `ShortId` branded type for their `shortId` fields to improve type safety:
+
+- **Database Layer**: The schema stores `shortId` fields as text columns in SQLite
+- **TypeScript Layer**: At compile time, these fields are typed as `ShortId` (a branded type) rather than plain strings
+- **Runtime Validation**: The `isShortId()` function validates that strings match the expected format (`/^[A-Za-z0-9_-]+$/`)
+- **Type Conversion**: The `asShortId()` function converts validated strings (such as route parameters) to the `ShortId` branded type
+- **Type Safety Benefits**: This branded type system prevents accidental mixing of different identifier types and provides compile-time validation
+
+This is a compile-time type safety improvement that doesn't change runtime behavior or database storage—shortId values remain text fields in the database.
+
 ### Automatic Initialization Features
 The database initialization process is fully automatic and requires no manual intervention:
 
@@ -115,19 +126,19 @@
 During startup, Zerobyte [ensures all entities have the latest configuration schema](https://github.com/nicotsx/zerobyte/blob/fdd8edec702ac43721ceffce5ac9af5cf4e545d5/app/server/modules/lifecycle/startup.ts#L17-L47). This process:
 
 - Updates volumes, repositories, and notification destinations to match current schema definitions
-- Identifies entities using their `shortId` (a branded type for type safety) rather than numeric IDs when calling service methods
+- Identifies entities using their `shortId` field (typed as `ShortId` branded type) when calling service methods
 - Uses `withContext` to set organization context for multi-tenancy support
 - Processes each entity independently—failures for individual entities are logged but don't halt startup
 - Ensures backward compatibility as configuration schemas evolve
 
-The system calls `volumeService.updateVolume(volume.shortId, volume)` and `repositoriesService.updateRepository(repo.shortId, {})` to update each entity's configuration schema.
+The system calls `volumeService.updateVolume(volume.shortId, volume)` and `repositoriesService.updateRepository(repo.shortId, {})`, where `volume.shortId` and `repo.shortId` are `ShortId` branded types that provide compile-time type safety when passing identifiers to service methods.
 
 ### Volume Auto-Remounting
 To restore the pre-shutdown state, Zerobyte [automatically remounts volumes during startup](https://github.com/nicotsx/zerobyte/blob/fdd8edec702ac43721ceffce5ac9af5cf4e545d5/app/server/modules/lifecycle/startup.ts#L57-L74):
 
 - **Target Selection**: Remounts volumes with status "mounted" OR volumes with `autoRemount: true` and status "error"
 - **Backend Support**: Handles NFS, SMB, WebDAV, SFTP, and local directory backends
-- **ShortId Identification**: Volumes are remounted by calling `volumeService.mountVolume(volume.shortId)` using the shortId branded type
+- **ShortId Identification**: Volumes are identified and remounted using their `shortId` field (typed as `ShortId` branded type) by calling `volumeService.mountVolume(volume.shortId)`
 - **Non-Blocking**: Each volume mount is wrapped in a `.catch()` handler—mount failures don't prevent application startup or affect other volumes
 - **One-Time Attempt**: Each volume receives exactly one mount attempt during startup; continuous retry is handled by the scheduled `VolumeAutoRemountJob`
 
@@ -221,7 +232,7 @@
 
 - **Schedule**: [Executes every 5 minutes](https://github.com/nicotsx/zerobyte/blob/fdd8edec702ac43721ceffce5ac9af5cf4e545d5/app/server/modules/lifecycle/startup.ts#L92) after startup
 - **Target Selection**: Only processes volumes with status "error" and `autoRemount: true`
-- **ShortId Usage**: Calls `volumeService.mountVolume(volume.shortId)` to remount volumes using the shortId branded type
+- **ShortId Usage**: Calls `volumeService.mountVolume(volume.shortId)` to remount volumes, where `volume.shortId` is a `ShortId` branded type
 - **Per-Volume Error Handling**: [Each volume mount is wrapped in try-catch](https://github.com/nicotsx/zerobyte/blob/fdd8edec702ac43721ceffce5ac9af5cf4e545d5/app/server/jobs/auto-remount.ts#L17-L23) to prevent one failure from affecting others
 - **Job-Level Error Handling**: [The scheduler wraps all job execution in error handlers](https://github.com/nicotsx/zerobyte/blob/fdd8edec702ac43721ceffce5ac9af5cf4e545d5/app/server/core/scheduler.ts#L22-L26), logging failures without stopping the scheduler
 - **Fixed Interval Strategy**: Uses a 5-minute fixed interval rather than exponential backoff, prioritizing consistent recovery attempts over reducing load

^{How did I do? Any feedback?}  

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

app/server/modules/volumes/__tests__/volumes.service.test.ts (1)

119-128: ⚠️ Potential issue | 🔴 Critical

Missing await — path-traversal rejection is never asserted.

Same problem as above: the expect(...).rejects.toThrow(...) promise is not awaited, so the security test for path traversal always passes trivially, providing no actual protection.

🐛 Proposed fix

-				expect(volumeService.listFiles(volume.shortId, traversalPath)).rejects.toThrow("Invalid path");
+				await expect(volumeService.listFiles(volume.shortId, traversalPath)).rejects.toThrow("Invalid path");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/server/modules/volumes/__tests__/volumes.service.test.ts` around lines
119 - 128, The test constructs a traversalPath and calls volumeService.listFiles
inside withContext but never awaits the expect(...).rejects.toThrow assertion,
so the rejection is not asserted; update the test to await the promise-based
assertion (i.e., await expect(volumeService.listFiles(volume.shortId,
traversalPath)).rejects.toThrow("Invalid path")) inside the withContext block so
the path-traversal rejection is actually verified for the listFiles call.

🧹 Nitpick comments (2)

app/server/utils/branded.ts (1)

1-9: Clean branded-type implementation.

The pattern is standard and the isShortId regex correctly matches the base64url alphabet produced by generateShortId.

One observation: asShortId is an unchecked cast used on user-supplied route params throughout the service layer (e.g., asShortId(String(data.volumeId))). While this is safe because unmatched values simply yield a NotFoundError from the DB, consider using isShortId at controller boundaries to reject malformed input early with a 400 Bad Request instead of hitting the database. This would also give isShortId a concrete use-site.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/server/utils/branded.ts` around lines 1 - 9, Controller code is currently
casting user-supplied route params to ShortId via asShortId without validation;
update controller-level request handling to validate incoming id strings with
isShortId before casting, returning a 400 Bad Request for malformed values
instead of passing invalid ShortId into the service/DB layer. Search for uses of
asShortId (and places converting route params like String(data.volumeId)) and
replace with a guard that calls isShortId(value) then casts to ShortId (or
returns a 400 error) so only validated values reach service functions that
expect ShortId. Ensure you reference the exported symbols isShortId, asShortId
and the ShortId type when making these changes.

app/server/jobs/healthchecks.ts (1)

19-21: LGTM on the shortId migration.

The surrounding withContext block lacks a try/catch, unlike RepositoryHealthCheckJob and VolumeAutoRemountJob which both guard their respective per-entity calls. If checkHealth or mountVolume throws (e.g., volume deleted between the initial query and the health check call), the loop aborts for all subsequent volumes. Worth aligning with the pattern used by its sibling jobs.

♻️ Align error handling with sibling jobs

 for (const volume of volumes) {
-    await withContext({ organizationId: volume.organizationId }, async () => {
-        const { status } = await volumeService.checkHealth(volume.shortId);
-        if (status === "error" && volume.autoRemount) {
-            await volumeService.mountVolume(volume.shortId);
-        }
-    });
+    try {
+        await withContext({ organizationId: volume.organizationId }, async () => {
+            const { status } = await volumeService.checkHealth(volume.shortId);
+            if (status === "error" && volume.autoRemount) {
+                await volumeService.mountVolume(volume.shortId);
+            }
+        });
+    } catch (error) {
+        logger.error(`Health check failed for volume ${volume.name}:`, error);
+    }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/server/jobs/healthchecks.ts` around lines 19 - 21, The withContext loop
calling volumeService.checkHealth and volumeService.mountVolume should be
wrapped in a per-volume try/catch (same pattern as RepositoryHealthCheckJob and
VolumeAutoRemountJob) so a thrown error for one volume doesn’t abort the whole
loop; locate the block where withContext invokes
volumeService.checkHealth(volume.shortId) and, around that call and the
subsequent mountVolume call, add a try/catch that logs the error (including
volume.shortId for context) and continues to the next volume.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/server/modules/backups/backups.service.ts`:
- Around line 44-45: Multiple queries in backups.service.ts use the verbose
drizzle-orm equality form (e.g., { shortId: { eq: shortId } }) mixed with
implicit shorthand ({ organizationId }); update all those filters to the concise
implicit form to be consistent: replace occurrences like { shortId: { eq:
shortId } } with { shortId } (and similarly for other fields listed in the
comment) in the methods using db.query.backupSchedulesTable.findFirst / findMany
/ update / delete so all conditional objects use the implicit equality
shorthand.

In `@app/server/modules/volumes/__tests__/volumes.service.test.ts`:
- Around line 84-90: The test "should throw NotFoundError for non-existent
volume" currently calls
expect(volumeService.getVolume(asShortId("nonexistent"))).rejects.toThrow(...)
without awaiting the assertion; change it to await
expect(volumeService.getVolume(asShortId("nonexistent"))).rejects.toThrow("Volume
not found") (or return the promise) so the rejection assertion on
volumeService.getVolume is actually evaluated inside the withContext block that
uses createTestSession and asShortId.

---

Outside diff comments:
In `@app/server/modules/volumes/__tests__/volumes.service.test.ts`:
- Around line 119-128: The test constructs a traversalPath and calls
volumeService.listFiles inside withContext but never awaits the
expect(...).rejects.toThrow assertion, so the rejection is not asserted; update
the test to await the promise-based assertion (i.e., await
expect(volumeService.listFiles(volume.shortId,
traversalPath)).rejects.toThrow("Invalid path")) inside the withContext block so
the path-traversal rejection is actually verified for the listFiles call.

---

Nitpick comments:
In `@app/server/jobs/healthchecks.ts`:
- Around line 19-21: The withContext loop calling volumeService.checkHealth and
volumeService.mountVolume should be wrapped in a per-volume try/catch (same
pattern as RepositoryHealthCheckJob and VolumeAutoRemountJob) so a thrown error
for one volume doesn’t abort the whole loop; locate the block where withContext
invokes volumeService.checkHealth(volume.shortId) and, around that call and the
subsequent mountVolume call, add a try/catch that logs the error (including
volume.shortId for context) and continues to the next volume.

In `@app/server/utils/branded.ts`:
- Around line 1-9: Controller code is currently casting user-supplied route
params to ShortId via asShortId without validation; update controller-level
request handling to validate incoming id strings with isShortId before casting,
returning a 400 Bad Request for malformed values instead of passing invalid
ShortId into the service/DB layer. Search for uses of asShortId (and places
converting route params like String(data.volumeId)) and replace with a guard
that calls isShortId(value) then casts to ShortId (or returns a 400 error) so
only validated values reach service functions that expect ShortId. Ensure you
reference the exported symbols isShortId, asShortId and the ShortId type when
making these changes.