From 233a912cbeb9668a3d97fda3e024470c5cf07c3a Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 04:41:13 -0700 Subject: [PATCH] test(provision): direct unit tests for readOrLazyHealInboundSecret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper landed in #2376 and is exercised via chat_files + registry integration tests. Those tests conflate the helper's behavior with the caller's response shape — a future refactor that broke the (secret, healed, err) contract subtly (e.g. returning healed=true on a read-success path, or swallowing a mint error) might still pass them. Adds 4 direct sub-tests pinning each branch of the contract: - secret already present → (s, false, nil) - secret missing, mint succeeds → (minted, true, nil) - secret missing, mint fails → ("", false, err) - read fails (non-NoInboundSecret) → ("", false, err) Each sub-case asserts the return tuple shape AND mock.ExpectationsWereMet (for the success path) so a future helper change that skips a DB op trips the gate immediately. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../workspace_provision_shared_test.go | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/workspace-server/internal/handlers/workspace_provision_shared_test.go b/workspace-server/internal/handlers/workspace_provision_shared_test.go index 2fd454b4..07a49c11 100644 --- a/workspace-server/internal/handlers/workspace_provision_shared_test.go +++ b/workspace-server/internal/handlers/workspace_provision_shared_test.go @@ -19,6 +19,7 @@ package handlers import ( "context" + "database/sql" "go/ast" "go/parser" "go/token" @@ -291,3 +292,104 @@ func TestPrepareProvisionContext_ParentIDInjection(t *testing.T) { } func ptrStr(s string) *string { return &s } + +// TestReadOrLazyHealInboundSecret pins the four branches of the +// shared lazy-heal helper directly. Each call site (chat_files, +// registry) has its own integration test, but those go through the +// public handlers and conflate the helper's behavior with the +// caller's response shape. This direct test pins the (secret, healed, +// err) contract on its own so a future refactor that breaks the +// helper signal — e.g., returning healed=true on a read-success path, +// or swallowing a mint error — fails immediately. +// +// The four branches: +// +// 1. Secret already present → (s, false, nil) +// 2. Secret missing, mint succeeds → (minted, true, nil) +// 3. Secret missing, mint fails → ("", false, mint-err) +// 4. Read fails (non-NoInboundSecret) → ("", false, read-err) +func TestReadOrLazyHealInboundSecret(t *testing.T) { + t.Run("secret already present → no heal, no error", func(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT platform_inbound_secret FROM workspaces WHERE id = \$1`). + WithArgs("ws-1"). + WillReturnRows(sqlmock.NewRows([]string{"platform_inbound_secret"}).AddRow("present-secret")) + + secret, healed, err := readOrLazyHealInboundSecret(context.Background(), "ws-1", "TestOp") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if secret != "present-secret" { + t.Errorf("secret: got %q, want %q", secret, "present-secret") + } + if healed { + t.Errorf("healed should be false when secret was already present") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unexpected sqlmock state — read happened but mint should NOT have: %v", err) + } + }) + + t.Run("secret missing → mint succeeds → returns healed=true", func(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT platform_inbound_secret FROM workspaces WHERE id = \$1`). + WithArgs("ws-2"). + WillReturnRows(sqlmock.NewRows([]string{"platform_inbound_secret"}).AddRow(nil)) + mock.ExpectExec(`UPDATE workspaces SET platform_inbound_secret = \$1 WHERE id = \$2`). + WithArgs(sqlmock.AnyArg(), "ws-2"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + secret, healed, err := readOrLazyHealInboundSecret(context.Background(), "ws-2", "TestOp") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if secret == "" { + t.Error("expected a freshly-minted secret string, got empty") + } + if !healed { + t.Error("healed should be true after lazy-heal mint succeeded") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations not met — mint did NOT run: %v", err) + } + }) + + t.Run("secret missing → mint fails → returns err and not healed", func(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT platform_inbound_secret FROM workspaces WHERE id = \$1`). + WithArgs("ws-3"). + WillReturnRows(sqlmock.NewRows([]string{"platform_inbound_secret"}).AddRow(nil)) + mock.ExpectExec(`UPDATE workspaces SET platform_inbound_secret = \$1 WHERE id = \$2`). + WithArgs(sqlmock.AnyArg(), "ws-3"). + WillReturnError(sql.ErrConnDone) + + secret, healed, err := readOrLazyHealInboundSecret(context.Background(), "ws-3", "TestOp") + if err == nil { + t.Fatal("expected mint error, got nil") + } + if secret != "" { + t.Errorf("expected empty secret on mint failure, got %q", secret) + } + if healed { + t.Error("healed must be false when mint failed") + } + }) + + t.Run("read fails (non-NoInboundSecret) → returns err and not healed", func(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT platform_inbound_secret FROM workspaces WHERE id = \$1`). + WithArgs("ws-4"). + WillReturnError(sql.ErrConnDone) + + secret, healed, err := readOrLazyHealInboundSecret(context.Background(), "ws-4", "TestOp") + if err == nil { + t.Fatal("expected read error, got nil") + } + if secret != "" { + t.Errorf("expected empty secret on read failure, got %q", secret) + } + if healed { + t.Error("healed must be false when read failed") + } + }) +}