Three small hardening passes from #2872's optional/important findings,
batched into one polish PR:
1. errors.Is(err, sql.ErrNoRows) instead of err == sql.ErrNoRows.
The bare equality breaks if any future caller wraps the error via
fmt.Errorf("…: %w", err) — the no-rows happy path would fall
through to the "real DB error" branch and abort the import.
errors.Is unwraps. New test
TestLookupExistingChild_WrappedNoRows_TreatedAsNotFound pins the
fix; verified the test fails on the old `==` shape (build break
on unused-import + assertion failure once import dropped).
2. Bounded 5s timeout on lookupExistingChild instead of
context.Background().
The createWorkspaceTree call site runs in goroutines spawned from
the /org/import handler, so plumbing the request context here
would cascade-cancel into provisionWorkspaceAuto and abort
in-flight EC2 provisioning if the client disconnected mid-import
— that's the wrong tradeoff. A short bounded timeout protects the
per-row SELECT against a wedged DB without taking the
drop-everything-on-disconnect behaviour. The lookup is a single
~10ms query; 5s leaves 500x headroom for transient slow paths.
3. Godoc clarifications on the skip-path block.
- /org/import is ADDITIVE-ONLY, never destructive. Children
present in the existing tree but absent from the new template
are preserved (no DELETE on diff).
- Skip-path does NOT propagate updates to existing nodes — a
re-import that adds an initial_memory or schedule to an
existing workspace is silently dropped. Document the limitation
so future operators know to delete-and-re-import or reach for
a future /org/sync route.
Verification:
- go build ./... → clean
- go test ./internal/handlers/... → all passing (TestLookup* +
TestCreateWorkspaceTree* + TestClass1* + TestGate*)
- 4 lookup tests + 1 new wrap-safety test → 5/5 PASS
- Full handlers suite → green
Refs molecule-core#2872 (Optional findings — wrap-safety + ctx, godoc
clarifications for additive-only + skip-path-update-limitation)
Out of scope (deferred):
- PR-D partial unique index migration + ON CONFLICT — sequenced
after Phase 4 cleanup verified clean per #2872 plan
- PR-E full createWorkspaceTree integration test for partial-match
— needs heavier sqlmock scaffolding for downstream
workspaces_audit/canvas_layouts/secrets/channels INSERTs;
follow-up
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
423d58d42c