Skip to content

fix: remove unique constraint from workspace name in schema and tests#113

Merged
khaliqgant merged 2 commits intomainfrom
fix/workspace-name-schema
Mar 31, 2026
Merged

fix: remove unique constraint from workspace name in schema and tests#113
khaliqgant merged 2 commits intomainfrom
fix/workspace-name-schema

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

@khaliqgant khaliqgant commented Mar 31, 2026

Summary

Part 1 of 2 — deploy this first, then the engine check removal.

PR #76 added migration 0003_workspace_name_not_globally_unique.sql to drop the unique index, but the Drizzle schema and test SQL weren't updated:

  • packages/server/src/db/schema.ts — remove .unique() from name column
  • packages/server/src/__tests__/directory.test.ts — remove UNIQUE from inline SQL
  • packages/server/src/__tests__/routing.test.ts — remove UNIQUE from inline SQL

Test plan

  • CI tests pass
  • Drizzle schema matches production DB (migration already applied)

🤖 Generated with Claude Code


Open with Devin

PR #76 added migration 0003 to drop the unique index on workspaces.name
but didn't update the Drizzle schema or test SQL. This aligns both.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
khaliqgant added a commit that referenced this pull request Mar 31, 2026
Part 2 of 2 (depends on #113 for schema alignment).

PR #78 re-introduced the check that PR #77 removed. The DB unique
constraint was already dropped in PR #76's migration. This removes
the redundant engine-level precheck.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment thread packages/server/src/db/schema.ts
export const workspaces = sqliteTable('workspaces', {
id: text('id').primaryKey(),
name: text('name').notNull().unique(),
name: text('name').notNull(),
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Stale application-level uniqueness check in createWorkspace contradicts migration intent

The migration at packages/server/src/db/migrations/0009_drop_workspace_name_unique.sql:2 explicitly states "Workspace names are NOT globally unique — multiple workspaces can share a name." However, createWorkspace at packages/server/src/engine/workspace.ts:10-18 still checks for an existing workspace with the same name and throws a 409 error, preventing duplicate-named workspaces from being created via POST /workspaces. This contradicts the PR's stated intent. Additionally, without the DB unique index backing it, this application-level check now has a TOCTOU race condition — two concurrent POST /workspaces requests with the same name can both pass the check and create duplicates.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@github-actions
Copy link
Copy Markdown

Preview deployed!

Environment URL
API https://pr113-api.relaycast.dev
Health https://pr113-api.relaycast.dev/health
Observer https://pr113-observer.relaycast.dev/observer

This preview shares the staging database and will be cleaned up when the PR is merged or closed.

Run E2E tests

npm run e2e -- https://pr113-api.relaycast.dev --ci

Open observer dashboard

https://pr113-observer.relaycast.dev/observer

PR #76 updated the schema intent but never added the actual migration.
The unique index from migration 0000 was still enforced in production.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
khaliqgant added a commit that referenced this pull request Mar 31, 2026
Part 2 of 2 (depends on #113 for schema alignment).

PR #78 re-introduced the check that PR #77 removed. The DB unique
constraint was already dropped in PR #76's migration. This removes
the redundant engine-level precheck.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

export const workspaces = sqliteTable('workspaces', {
id: text('id').primaryKey(),
name: text('name').notNull().unique(),
name: text('name').notNull(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Removing unique constraint makes getWorkspaceByName and A2A workspace resolution non-deterministic

With the UNIQUE constraint removed from workspaces.name, duplicate workspace names can now be created via PATCH /workspace (the updateWorkspace function at packages/server/src/engine/workspace.ts:88-118 has no application-level name uniqueness check — it relied entirely on the DB constraint). Once duplicates exist, getWorkspaceByName (packages/server/src/engine/workspace.ts:53-68) will silently return an arbitrary matching workspace. This affects the public GET /workspaces/by-name/:name endpoint and, more critically, the A2A handleWorkspaceAgentCard function at packages/server/src/routes/a2a.ts:245-249, which resolves a workspace by name to serve its agent card — potentially exposing the wrong workspace's agent data to an unauthenticated caller.

Prompt for agents
The unique constraint on workspaces.name was removed, but multiple code paths still assume workspace names are globally unique. You need to either:

1. Update all workspace-by-name lookups to handle ambiguity (return an error or a list instead of a single arbitrary result). Affected locations:
   - packages/server/src/engine/workspace.ts:53-68 (getWorkspaceByName) — used by GET /workspaces/by-name/:name
   - packages/server/src/routes/a2a.ts:245-249 (handleWorkspaceAgentCard) — resolves workspace for A2A agent card

2. OR add application-level uniqueness enforcement in updateWorkspace (packages/server/src/engine/workspace.ts:88-118) to prevent PATCH /workspace from creating duplicate names, similar to the existing check in createWorkspace.

Option 2 is simpler but contradicts the migration comment that says names are NOT globally unique. If the intent truly is to allow duplicate names, option 1 is needed, and these endpoints may need to be changed to use workspace ID instead of name.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@khaliqgant khaliqgant merged commit 9f6a2c9 into main Mar 31, 2026
4 checks passed
@khaliqgant khaliqgant deleted the fix/workspace-name-schema branch March 31, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant