perf(workspace): shorten model-facing workspace IDs - #129
Conversation
📝 WalkthroughWalkthroughWorkspace IDs now use 5 random bytes encoded as hexadecimal. The checkout test verifies the ChangesWorkspace ID generation
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR shortens newly generated workspace IDs from UUIDs to a
Confidence Score: 4/5The PR appears safe to merge, though collision recovery would make the deliberately reduced identifier space more robust for long-lived installations. The new format behaves as intended, but a duplicate 40-bit ID would currently fail persistence or overwrite an in-memory workspace rather than being regenerated. Files Needing Attention: src/workspaces.ts
|
| Filename | Overview |
|---|---|
| src/workspaces.ts | Changes workspace IDs to a 40-bit random format, but does not add collision detection or retry handling. |
| src/workspaces.test.ts | Verifies that newly opened workspaces use the expected compact ID format. |
Reviews (1): Last reviewed commit: "perf(workspace): shorten workspace ids" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }): Promise<WorkspaceContext> { | ||
| const workspace: Workspace = { | ||
| id: `ws_${randomUUID()}`, | ||
| id: `ws_${randomBytes(5).toString("hex")}`, |
There was a problem hiding this comment.
The new 40-bit ID space has no collision retry: a duplicate causes the persisted workspace insert to fail with a primary-key error, while registries without a store silently replace the existing workspace in the in-memory map.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/workspaces.ts`:
- Line 358: Update workspace creation around the ID generation in the workspace
factory to prevent collisions from overwriting existing entries or sessions. Use
a larger collision-safe ID namespace, or atomically check the generated ID
against this.workspaces and retry before inserting and returning the workspace;
preserve the existing workspace and session data when a collision occurs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d3d5664-d5dc-4013-aafa-43c81b0f4900
📒 Files selected for processing (2)
src/workspaces.test.tssrc/workspaces.ts
| }): Promise<WorkspaceContext> { | ||
| const workspace: Workspace = { | ||
| id: `ws_${randomUUID()}`, | ||
| id: `ws_${randomBytes(5).toString("hex")}`, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow static proxy. It does not run packaged npm/npx installs or MCP hosts.
rg -n --glob '!node_modules/**' \
-P '\b(createSession|getSession|getWorkspace|workspaceId|WorkspaceStore|randomUUID|randomBytes)\b' .Repository: Waishnav/devspace
Length of output: 28095
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/workspace-store.ts outline =="
ast-grep outline src/workspace-store.ts --view expanded || true
echo "== src/workspace-store.ts relevant sections =="
sed -n '1,140p' src/workspace-store.ts
sed -n '180,225p' src/workspace-store.ts
echo "== src/workspaces.ts outline =="
ast-grep outline src/workspaces.ts --view expanded || true
echo "== src/workspaces.ts relevant sections =="
sed -n '1,120p' src/workspaces.ts
sed -n '230,390p' src/workspaces.ts
echo "== deterministic capacity/collision proxy =="
python3 - <<'PY'
from math import sqrt
import math
space = 2**40
# approximate birthday median / sqrt(2*N*ln(2)) when p=0.5
median = int(sqrt(2 * space * math.log(2))) + 1
print(f"id_space={space}")
print(f"estimated_approximate_sessions_for_50_percent_collision={median}")
print(f"hex_bytes_generated={5}")
print(f"expected_replacements_per_unique_workspace={space/(space-median)**2:.1e}" if False else "")
# Show tiny probabilities for small n via Poisson approximation n^2/(2 space).
for n in [1, 10, 1_000, 10_000]:
n = int(n)
p = n*(n-1)/(2*space)
print(f"n={n}, expected_pairs={p:.2e}, p_collision_lower_bound_by_pairs={min(p,1):.6e}")
PYRepository: Waishnav/devspace
Length of output: 19695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/workspace-store.ts createSession full relevant block with line numbers =="
nl -ba src/workspace-store.ts | sed -n '66,88p'
echo "== database schema workspace session constraints =="
nl -ba src/db/schema.ts | sed -n '70,135p'
echo "== workspace ID consumers around collision handling =="
nl -ba src/workspaces.ts | sed -n '90,185p'
nl -ba src/server.test.ts | sed -n '60,180p' | sed -n '25,170p'Repository: Waishnav/devspace
Length of output: 276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/workspace-store.ts createSession full relevant block with line numbers =="
awk '{printf "%6d\t%s\n", NR, $0}' src/workspace-store.ts | sed -n '66,88p'
echo "== database schema workspace session constraints =="
awk '{printf "%6d\t%s\n", NR, $0}' src/db/schema.ts | sed -n '70,135p'
echo "== workspace ID consumers around collision handling =="
awk '{printf "%6d\t%s\n", NR, $0}' src/workspaces.ts | sed -n '90,185p'
awk '{printf "%6d\t%s\n", NR, $0}' src/server.test.ts | sed -n '60,180p'Repository: Waishnav/devspace
Length of output: 13915
Make the 40-bit workspace ID space collision-safe.
randomBytes(5).toString("hex") creates only 2^40 possible IDs. A collision can replace an existing workspace in this.workspaces.set(workspace.id, workspace) and can also overwrite the existing workspace session with the new workspace root/data. Use a collision-safe ID namespace, or enforce uniqueness atomically with retry before returning the workspace. The current tests do not cover ID collisions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workspaces.ts` at line 358, Update workspace creation around the ID
generation in the workspace factory to prevent collisions from overwriting
existing entries or sessions. Use a larger collision-safe ID namespace, or
atomically check the generated ID against this.workspaces and retry before
inserting and returning the workspace; preserve the existing workspace and
session data when a collision occurs.
1 participant
Workspace IDs are repeated across model-facing tool calls, and the current UUID-based values add unnecessary context overhead. New workspaces now use a compact
ws_prefix followed by 10 lowercase hex characters, while existing persisted IDs continue to work unchanged because IDs remain opaque strings.Summary by CodeRabbit
ws_followed by 10 lowercase hexadecimal characters.