refactor(store/pg): add parseUUID error variant

Two new helpers alongside existing mustParseUUID:

- parseUUID(s) (uuid.UUID, error) — use for INSERT/UPDATE/UPSERT/DELETE
  and SELECT-WHERE paths where silent nil hides bugs. Returns a wrapped
  error that includes the raw value (for debuggability) and the
  underlying uuid parse error (for errors.Is unwrap).

- parseUUIDOrNil(s) uuid.UUID — intentionally silent, for read-only
  SELECT-WHERE paths where empty result is acceptable.

mustParseUUID is kept as a transitional alias delegating to parseUUIDOrNil
so existing call sites compile unchanged. Sub-step 4d will rename it.

Phase 4 Step 4a of agent identity hardening (TD-1).
This commit is contained in:
viettranx committed 2026-04-11 21:22:23 +07:00
1 parent 6bbe1b66ff
commit 9a8eb396b0
2 files changed
+112 -1

No files matched your search

+28 -1
View File
@@ -464,7 +464,27 @@ func (s *PGMemoryStore) Close() error { return nil }
// --- Helpers ---
func mustParseUUID(s string) uuid.UUID {
// parseUUID returns the parsed UUID or a descriptive error.
// Use for every INSERT/UPDATE/UPSERT/DELETE/SELECT-WHERE path where silent nil
// would either corrupt data (FK constraints reject it, but callers see cryptic
// PG code 23503 instead of a clean Go error) OR hide bugs as empty reads /
// zero-row updates. Phase 4 migration target for all 57 CRITICAL sites.
// See docs/agent-identity-conventions.md (Phase 6).
func parseUUID(s string) (uuid.UUID, error) {
id, err := uuid.Parse(s)
if err != nil {
return uuid.Nil, fmt.Errorf("parse uuid %q: %w", s, err)
}
return id, nil
}
// parseUUIDOrNil returns the parsed UUID or uuid.Nil on failure, without
// raising an error. INTENTIONALLY silent — only use for read-only SELECT-WHERE
// paths where a no-match (empty result) is acceptable. Do NOT use for writes,
// updates, deletes, or any SELECT where empty result would hide a bug.
// Prefer parseUUID for any new code.
// See docs/agent-identity-conventions.md (Phase 6).
func parseUUIDOrNil(s string) uuid.UUID {
id, err := uuid.Parse(s)
if err != nil {
return uuid.Nil
@@ -472,6 +492,13 @@ func mustParseUUID(s string) uuid.UUID {
return id
}
// mustParseUUID is an alias for parseUUIDOrNil kept during the Phase 4
// migration window. Will be removed in sub-step 4d (single-commit rename).
// TODO(Phase 4 4d): remove after all call sites migrate.
func mustParseUUID(s string) uuid.UUID {
return parseUUIDOrNil(s)
}
func vectorToString(v []float32) string {
if len(v) == 0 {
return ""
+84
View File
@@ -0,0 +1,84 @@
package pg
import (
"errors"
"strings"
"testing"
"github.com/google/uuid"
)
func TestParseUUID_Valid(t *testing.T) {
want := uuid.New()
got, err := parseUUID(want.String())
if err != nil {
t.Fatalf("parseUUID(valid): unexpected error %v", err)
}
if got != want {
t.Errorf("parseUUID: got %v, want %v", got, want)
}
}
func TestParseUUID_Invalid_ReturnsError(t *testing.T) {
got, err := parseUUID("goctech-leader")
if err == nil {
t.Fatal("parseUUID(agent_key): expected error, got nil")
}
if got != uuid.Nil {
t.Errorf("parseUUID(invalid): got %v, want uuid.Nil", got)
}
// Error must include both the offending value (for debuggability) and wrap
// the underlying uuid package error (for errors.Is assertions).
if !strings.Contains(err.Error(), "goctech-leader") {
t.Errorf("parseUUID error must include raw value, got: %v", err)
}
}
func TestParseUUID_Empty_ReturnsError(t *testing.T) {
_, err := parseUUID("")
if err == nil {
t.Error("parseUUID(\"\"): expected error for empty input")
}
}
func TestParseUUIDOrNil_Valid(t *testing.T) {
want := uuid.New()
got := parseUUIDOrNil(want.String())
if got != want {
t.Errorf("parseUUIDOrNil: got %v, want %v", got, want)
}
}
func TestParseUUIDOrNil_Invalid_ReturnsNil(t *testing.T) {
got := parseUUIDOrNil("not-a-uuid")
if got != uuid.Nil {
t.Errorf("parseUUIDOrNil(invalid): got %v, want uuid.Nil", got)
}
}
// TestParseUUID_ErrorIsUnwrappable verifies the wrapped error chain so
// downstream callers can errors.Is/errors.As against the root uuid error.
func TestParseUUID_ErrorIsUnwrappable(t *testing.T) {
_, err := parseUUID("bad")
if err == nil {
t.Fatal("expected error")
}
// Unwrap should not be nil — the wrapped error chain is what lets callers
// decide whether to return a 400 vs 500.
if errors.Unwrap(err) == nil {
t.Error("parseUUID error should wrap the underlying uuid.Parse error")
}
}
// TestMustParseUUID_AliasToParseUUIDOrNil verifies the transitional alias
// during Phase 4 migration — mustParseUUID must behave identically to
// parseUUIDOrNil so existing call sites are untouched until rename (4d).
func TestMustParseUUID_AliasToParseUUIDOrNil(t *testing.T) {
u := uuid.New()
if mustParseUUID(u.String()) != u {
t.Error("mustParseUUID should equal parseUUIDOrNil on valid input")
}
if mustParseUUID("garbage") != uuid.Nil {
t.Error("mustParseUUID should return uuid.Nil on invalid input (parseUUIDOrNil semantics)")
}
}