forked from molecule-ai/molecule-core
da3cb4c098
## Bug
`/org/import` had no per-tenant mutex, advisory lock, or DB-level
uniqueness on (parent_id, name). The pattern was lookup-then-insert:
existingID, existing, err := h.lookupExistingChild(...) // SELECT
if existing { return /* skip */ }
db.DB.ExecContext(ctx, `INSERT INTO workspaces ...`) // INSERT
Two concurrent admin POSTs (rapid double-click in canvas, retry-after-
timeout, two operators on the same template) both saw "not found" in
the SELECT and both INSERT'd the same (parent_id, name).
Captured impact: tenant-hongming accumulated 72 stale child workspaces
in 4 days from repeated org-template spawns of the same template
(see #2857 phase 4 sweeper for the cleanup; #2872 for the prevention RFC).
## Fix
Two-layer fix — DB-level backstop AND application-level happy path:
1. **Migration** `20260506000000_workspaces_unique_parent_name.up.sql`
```sql
CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS workspaces_parent_name_uniq
ON workspaces (
COALESCE(parent_id, '00000000-0000-0000-0000-000000000000'::uuid),
name
)
WHERE status != 'removed';
```
* COALESCE(parent_id, sentinel) collapses NULLs so root workspaces
also collide pairwise.
* `WHERE status != 'removed'` lets a tombstoned row be replaced
by a same-named re-import (preserves existing org-import semantics).
* CONCURRENTLY avoids ACCESS EXCLUSIVE on production tenants under
live traffic; IF NOT EXISTS makes the migration resumable.
* Down migration drops CONCURRENTLY symmetrically.
2. **`org_import.go` swap**
Replace lookup-then-insert with `INSERT ... ON CONFLICT DO NOTHING
RETURNING id`. On the skip path (RETURNING returns 0 rows →
sql.ErrNoRows), re-select the existing id to recurse children:
INSERT INTO workspaces (...) VALUES (...)
ON CONFLICT (COALESCE(parent_id, ...), name)
WHERE status != 'removed'
DO NOTHING
RETURNING id;
The ON CONFLICT target predicate matches the partial-index predicate
exactly — required for Postgres to consider the index applicable.
Existing `lookupExistingChild` helper kept (still used on the skip
path); semantics unchanged.
## Test coverage
* AST gate refreshed to assert the workspaces INSERT contains the
ON CONFLICT pattern (`onConflictDoNothingRE`) instead of the now-obsolete
"lookup-before-insert" ordering. Per behavior-based gating
(memory: feedback_behavior_based_ast_gates.md), the new gate pins
the actual TOCTOU-resolution behavior.
* Companion `TestGate_FailsWhenInsertOmitsOnConflict` proves the gate
catches the bug shape on synthetic source.
* All existing `lookupExistingChild` unit tests (no-rows, found,
nil-parent, DB error, wrapped no-rows) still pass — helper is
unchanged and still load-bearing on the skip path.
* Live Postgres E2E coverage runs via the existing
"Handlers Postgres Integration" CI job, which applies migrations
to a real PG and exercises the INSERT path.
## Why ship the migration + swap together (not stacked)
The migration alone provides a DB-level backstop, but without the
handler swap a UNIQUE-violation surfaces as a 500 to the user. The
handler swap alone has no enforceable target until the migration
applies. Shipped together they give graceful skip + atomic backstop.
Migration is CONCURRENTLY + IF NOT EXISTS, safe to apply even on
tenants where the sweeper (#2860) hasn't run yet — the index just
declines to build until conflicting rows are reconciled.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|---|---|---|
| .. | ||
| 001_workspaces.sql | ||
| 002_agents.sql | ||
| 003_events.sql | ||
| 004_secrets.sql | ||
| 005_canvas_layouts.sql | ||
| 006_workspace_config_memory.sql | ||
| 007_approvals.sql | ||
| 008_agent_memories.sql | ||
| 009_activity_logs.sql | ||
| 010_workspace_awareness.sql | ||
| 011_workspace_runtime.sql | ||
| 012_global_secrets.sql | ||
| 013_workspace_dir.sql | ||
| 014_indexes.sql | ||
| 015_workspace_schedules.sql | ||
| 016_workspace_channels.sql | ||
| 017_memories_fts_namespace.down.sql | ||
| 017_memories_fts_namespace.up.sql | ||
| 018_secrets_encryption_version.down.sql | ||
| 018_secrets_encryption_version.up.sql | ||
| 019_workspace_access.down.sql | ||
| 019_workspace_access.up.sql | ||
| 020_workspace_auth_tokens.down.sql | ||
| 020_workspace_auth_tokens.up.sql | ||
| 021_delegation_idempotency.down.sql | ||
| 021_delegation_idempotency.up.sql | ||
| 022_workspace_schedules_source.down.sql | ||
| 022_workspace_schedules_source.up.sql | ||
| 023_workspace_memory_version.down.sql | ||
| 023_workspace_memory_version.up.sql | ||
| 024_channel_budget.down.sql | ||
| 024_channel_budget.up.sql | ||
| 025_workspace_token_usage.down.sql | ||
| 025_workspace_token_usage.up.sql | ||
| 026_org_plugin_allowlist.down.sql | ||
| 026_org_plugin_allowlist.up.sql | ||
| 027_workspace_budget.down.sql | ||
| 027_workspace_budget.up.sql | ||
| 028_workspace_artifacts.down.sql | ||
| 028_workspace_artifacts.up.sql | ||
| 029_workspace_hibernation.down.sql | ||
| 029_workspace_hibernation.up.sql | ||
| 030_audit_events.down.sql | ||
| 030_audit_events.up.sql | ||
| 031_memories_pgvector.down.sql | ||
| 031_memories_pgvector.up.sql | ||
| 032_schedule_consecutive_empty.down.sql | ||
| 032_schedule_consecutive_empty.up.sql | ||
| 033_strip_crlf_cron_prompts.up.sql | ||
| 034_workspaces_last_outbound_at.up.sql | ||
| 035_org_api_tokens.down.sql | ||
| 035_org_api_tokens.up.sql | ||
| 036_org_api_tokens_org_id.down.sql | ||
| 036_org_api_tokens_org_id.up.sql | ||
| 037_max_concurrent_tasks.down.sql | ||
| 037_max_concurrent_tasks.up.sql | ||
| 038_workspace_instance_id.down.sql | ||
| 038_workspace_instance_id.up.sql | ||
| 039_activity_tool_trace.down.sql | ||
| 039_activity_tool_trace.up.sql | ||
| 040_platform_instructions.down.sql | ||
| 040_platform_instructions.up.sql | ||
| 042_a2a_queue.down.sql | ||
| 042_a2a_queue.up.sql | ||
| 043_workspace_status_enum.down.sql | ||
| 043_workspace_status_enum.up.sql | ||
| 044_platform_inbound_secret.down.sql | ||
| 044_platform_inbound_secret.up.sql | ||
| 045_workspaces_delivery_mode.down.sql | ||
| 045_workspaces_delivery_mode.up.sql | ||
| 046_workspace_status_awaiting_agent.down.sql | ||
| 046_workspace_status_awaiting_agent.up.sql | ||
| 047_runtime_image_pins.down.sql | ||
| 047_runtime_image_pins.up.sql | ||
| 048_activity_logs_peer_indexes.down.sql | ||
| 048_activity_logs_peer_indexes.up.sql | ||
| 049_delegations.down.sql | ||
| 049_delegations.up.sql | ||
| 20260417000000_workflow_checkpoints.down.sql | ||
| 20260417000000_workflow_checkpoints.up.sql | ||
| 20260505100000_pending_uploads.down.sql | ||
| 20260505100000_pending_uploads.up.sql | ||
| 20260505200000_pending_uploads_acked_index.down.sql | ||
| 20260505200000_pending_uploads_acked_index.up.sql | ||
| 20260506000000_workspaces_unique_parent_name.down.sql | ||
| 20260506000000_workspaces_unique_parent_name.up.sql | ||