forked from molecule-ai/molecule-core
423d58d42c
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> |
||
|---|---|---|
| .. | ||
| cmd | ||
| internal | ||
| migrations | ||
| pkg/provisionhook | ||
| .ci-force | ||
| .gitignore | ||
| .golangci.yaml | ||
| Dockerfile | ||
| Dockerfile.tenant | ||
| entrypoint-tenant.sh | ||
| go.mod | ||
| go.sum | ||