From 47b7ccb002df104e902be6bab22f86c5290bba3c Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer B (MiniMax)" Date: Fri, 12 Jun 2026 06:32:53 +0000 Subject: [PATCH 1/3] test(platform-agent): regression for core#2496 ON CONFLICT updates runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CTO-mandated regression test for the core#2496 root-fix (commit 67c2fff3). The InstallPlatformAgent endpoint upserts the platform-agent row with runtime='claude-code', but the ON CONFLICT clause originally only updated kind and parent_id. If the platform-agent container self-registered first (creating a row with the schema default runtime), the install endpoint would NOT correct the runtime — it would be left stale. #2496 added `runtime = 'claude-code'` to the ON CONFLICT DO UPDATE SET clause, so the runtime is now corrected on every install, not just on the initial insert. This commit adds TestIntegration_PlatformAgentInstall_OnConflictUpdatesRuntime to platform_agent_integration_test.go. The test: 1. Seeds a platform-agent row with a deliberately STALE runtime ('codex') — simulates the self-registered row scenario. 2. Asserts the seed has the stale runtime (not 'claude-code'). If this fails, the test is meaningless — fix the seed, not the assertion. 3. Calls installPlatformAgent. Per the pre-#2496 bug, the ON CONFLICT DO UPDATE SET clause would have left runtime stuck at 'codex'. Per the #2496 fix, it MUST be 'claude-code'. 4. PRIMARY ASSERTION: post-install runtime is 'claude-code'. The error message names the exact pre-#2496 vs post-#2496 clause shape so a future regressor sees what to fix. 5. Idempotency: a second install still leaves runtime at 'claude-code' (the conflict path is hit on every call). 6. Fresh-insert path: also asserts runtime='claude-code' on a brand-new row. Guards against a future refactor that moves the runtime to ONLY the conflict clause and removes it from the INSERT VALUES — a regression that would let a self-registering row skip the runtime entirely. The test uses the existing integration build tag (`//go:build integration` + `// +build integration`) and the INTEGRATION_DB_URL env var, following the same pattern as the other tests in the file. Compiles cleanly; runs and is skipped locally without INTEGRATION_DB_URL; will exercise the real Postgres ON CONFLICT path in the handlers-postgres-integration CI workflow. Test results: ok workspace-server/internal/handlers (skip pattern correct) vet clean (with -tags=integration) Refs: molecule-core#2615, core#2496, mc#1982 Co-Authored-By: Claude --- .../platform_agent_integration_test.go | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/workspace-server/internal/handlers/platform_agent_integration_test.go b/workspace-server/internal/handlers/platform_agent_integration_test.go index c73f2777a..0e46b6a76 100644 --- a/workspace-server/internal/handlers/platform_agent_integration_test.go +++ b/workspace-server/internal/handlers/platform_agent_integration_test.go @@ -290,3 +290,123 @@ func TestIntegration_PlatformAgentInstall_StatusNotOnline(t *testing.T) { t.Logf("fresh platform-agent status=%q (expected 'offline', but accepting non-online)", status) } } + +// TestIntegration_PlatformAgentInstall_OnConflictUpdatesRuntime locks in +// the core#2496 root-fix. Pre-#2496, the ON CONFLICT clause on the +// platform-agent upsert only set kind and parent_id; the runtime column +// was left at its INSERT-time value (the schema default). So if a +// platform-agent row had been self-registered first (creating a row with +// the schema default runtime) and the install endpoint was then called, +// the runtime would be left STALE — not updated to 'claude-code'. +// +// #2496 added `runtime = 'claude-code'` to the ON CONFLICT DO UPDATE +// SET clause, so the runtime is now corrected on every install, not just +// on the initial insert. +// +// This test seeds a platform-agent row with a NON-canonical runtime +// ('codex' — anything other than 'claude-code'), then calls +// installPlatformAgent. The post-install assertion is: the runtime +// MUST be 'claude-code'. Pre-#2496, this assertion would fail; post-#2496, +// the ON CONFLICT clause resets it. +// +// We also test the "fresh insert" path (no prior row) to confirm the +// regression is specifically about the conflict-path; if a future refactor +// accidentally drops the runtime from the INSERT VALUES too, that case +// catches it. +func TestIntegration_PlatformAgentInstall_OnConflictUpdatesRuntime(t *testing.T) { + conn := integrationDB_PlatformAgentInstall(t) + ctx := context.Background() + + tag := uuid.New().String()[:8] + platformID := uuid.New().String() + paName := "Org Concierge " + tag + staleRuntime := "codex" // deliberately non-canonical + + cleanup := func() { + _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE name = $1`, paName) + _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, platformID) + } + t.Cleanup(cleanup) + cleanup() + + // Case 1: ON CONFLICT path. Seed the platform-agent row with a STALE + // runtime (simulates a self-registered row with the schema default). + // The seed is INSERT (not UPSERT) to deliberately NOT use the install + // endpoint's path — we're pre-arranging the conflict scenario. + if _, err := conn.ExecContext(ctx, ` + INSERT INTO workspaces (id, name, kind, tier, status, runtime, parent_id) + VALUES ($1, $2, 'platform', 0, 'offline', $3, NULL)`, + platformID, paName, staleRuntime); err != nil { + t.Fatalf("seed stale platform agent: %v", err) + } + + // Sanity: the seeded row has the stale runtime (not the canonical + // 'claude-code'). If this assertion fails, the seed is wrong; fix + // the seed, not the test assertion. + var seededRuntime string + if err := conn.QueryRowContext(ctx, + `SELECT runtime FROM workspaces WHERE id = $1`, platformID).Scan(&seededRuntime); err != nil { + t.Fatalf("read seeded runtime: %v", err) + } + if seededRuntime != staleRuntime { + t.Fatalf("seed precondition: runtime=%q, want %q (the test would be meaningless if the seed already had 'claude-code')", + seededRuntime, staleRuntime) + } + + // Now call installPlatformAgent. Per the pre-#2496 bug, the + // ON CONFLICT DO UPDATE SET clause would have left runtime stuck + // at 'codex'. Per the #2496 fix, it must now be 'claude-code'. + if err := installPlatformAgent(ctx, conn, platformID, paName); err != nil { + t.Fatalf("install: %v", err) + } + + // PRIMARY ASSERTION: the runtime is 'claude-code' after install. + // Pre-#2496, the runtime would be 'codex' (stale). Post-#2496, the + // ON CONFLICT clause sets it to 'claude-code'. + var postInstallRuntime string + if err := conn.QueryRowContext(ctx, + `SELECT runtime FROM workspaces WHERE id = $1`, platformID).Scan(&postInstallRuntime); err != nil { + t.Fatalf("read post-install runtime: %v", err) + } + if postInstallRuntime != "claude-code" { + t.Fatalf("core#2496 regression: post-install runtime=%q, want 'claude-code'. "+ + "This means the ON CONFLICT DO UPDATE SET clause is NOT setting runtime on the conflict path. "+ + "Pre-#2496, the clause was: `ON CONFLICT (id) DO UPDATE SET kind = 'platform', parent_id = NULL` "+ + "(no runtime update). Post-#2496, it MUST be: `... SET kind = 'platform', runtime = 'claude-code', parent_id = NULL`.", + postInstallRuntime) + } + + // Sanity: a second install (still the conflict path) keeps runtime + // at 'claude-code' (idempotency assertion). + if err := installPlatformAgent(ctx, conn, platformID, paName); err != nil { + t.Fatalf("second install: %v", err) + } + var post2Runtime string + if err := conn.QueryRowContext(ctx, + `SELECT runtime FROM workspaces WHERE id = $1`, platformID).Scan(&post2Runtime); err != nil { + t.Fatalf("read post-second-install runtime: %v", err) + } + if post2Runtime != "claude-code" { + t.Fatalf("idempotent: post-second-install runtime=%q, want 'claude-code'", post2Runtime) + } + + // Case 2: Fresh insert path. Verify the runtime='claude-code' is + // also set on the INSERT VALUES, not just on conflict. This guards + // against a future refactor that moves the runtime to the conflict + // clause only and removes it from the INSERT — a regression that + // would let a self-registering row skip the runtime entirely. + freshID := uuid.New().String() + freshName := "Org Concierge fresh " + tag + if err := installPlatformAgent(ctx, conn, freshID, freshName); err != nil { + t.Fatalf("fresh install: %v", err) + } + var freshRuntime string + if err := conn.QueryRowContext(ctx, + `SELECT runtime FROM workspaces WHERE id = $1`, freshID).Scan(&freshRuntime); err != nil { + t.Fatalf("read fresh runtime: %v", err) + } + if freshRuntime != "claude-code" { + t.Fatalf("fresh insert path: runtime=%q, want 'claude-code'", freshRuntime) + } + _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, freshID) +} -- 2.52.0 From 9227e2d48da6ce727b624fd75df33d3feff80190 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer B (MiniMax)" Date: Fri, 12 Jun 2026 07:17:22 +0000 Subject: [PATCH 2/3] =?UTF-8?q?test(platform-agent):=20fix=20OnConflict=20?= =?UTF-8?q?test=20isolation=20=E2=80=94=20pre-seed=20nukes=20the=20uniq-pl?= =?UTF-8?q?atform-root=20slot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Handlers Postgres Integration CI job for PR #2629 was failing on TestIntegration_PlatformAgentInstall_OnConflictUpdatesRuntime with: seed stale platform agent: pq: duplicate key value violates unique constraint "uniq_workspaces_one_platform_root" Root cause: the schema has uniq_workspaces_one_platform_root, a UNIQUE partial index allowing at most ONE row with kind='platform' AND parent_id IS NULL per database. A prior sibling test in this package (TestIntegration_PlatformAgentInstall_ ReparentsRootAndMovesAnchors and friends) leaves a platform-agent row behind; its cleanup deletes by its OWN uuid+name, so by the time my test runs, the slot may be occupied by a prior test's row (depending on test scheduling). My test was seeding kind='platform' and getting blocked by the constraint. Fix: the pre-seed cleanup nuke ANY existing kind='platform' AND parent_id IS NULL row (any uuid, any name) before my test seeds. The slot is single-tenant by design, so this is safe in a fresh CI database (this test is the only writer of kind='platform' rows; sub-tests' cleanups remove their own at the end of their run, and the constraint guarantees at most one ever exists). The post-test cleanup also gets a belt-and-suspenders delete of any other platform-agent row to keep the next test's slot free. Refs: molecule-core#2615, core#2496, PR #2629 Co-Authored-By: Claude --- .../platform_agent_integration_test.go | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/workspace-server/internal/handlers/platform_agent_integration_test.go b/workspace-server/internal/handlers/platform_agent_integration_test.go index 0e46b6a76..a6e1f5802 100644 --- a/workspace-server/internal/handlers/platform_agent_integration_test.go +++ b/workspace-server/internal/handlers/platform_agent_integration_test.go @@ -322,11 +322,37 @@ func TestIntegration_PlatformAgentInstall_OnConflictUpdatesRuntime(t *testing.T) paName := "Org Concierge " + tag staleRuntime := "codex" // deliberately non-canonical + // The schema has `uniq_workspaces_one_platform_root` (UNIQUE + // constraint allowing at most one row with kind='platform' AND + // parent_id IS NULL per database). A prior sibling test in this + // package (e.g. TestIntegration_PlatformAgentInstall_ReparentsRoot + // AndMovesAnchors) leaves a platform-agent row behind; its cleanup + // deletes by its OWN uuid+name, so by the time this test runs, + // the slot may or may not be free depending on test scheduling and + // whether the prior cleanup ran. + // + // To make this test robust to test ordering and to the + // existing-test-row state, the pre-seed cleanup nukes ANY existing + // platform-agent row (any uuid, any name) — the slot is + // single-tenant by design, so this is safe in a fresh CI database + // (this is the only writer of kind='platform' rows; sub-tests' + // cleanup removes them at the end of their own run). The + // post-test cleanup then removes our specific rows. cleanup := func() { _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE name = $1`, paName) _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, platformID) + // Belt-and-suspenders: also drop any other platform-agent row + // left by a prior test that somehow didn't clean up, so the + // next test's seed has a free slot. + _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE kind = 'platform' AND parent_id IS NULL AND id <> $1`, platformID) } t.Cleanup(cleanup) + // Pre-seed: clear the slot completely (nuke any pre-existing + // platform-agent row, including the unique-constraint violating one + // from a prior sibling test). + if _, err := conn.ExecContext(ctx, `DELETE FROM workspaces WHERE kind = 'platform' AND parent_id IS NULL`); err != nil { + t.Fatalf("pre-seed: delete existing platform-agent rows: %v", err) + } cleanup() // Case 1: ON CONFLICT path. Seed the platform-agent row with a STALE -- 2.52.0 From 4ad8f22f5f45f4be20383dd20a5f839f60d4fafa Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer B (MiniMax)" Date: Fri, 12 Jun 2026 07:21:22 +0000 Subject: [PATCH 3/3] =?UTF-8?q?test(platform-agent):=20fix=20OnConflict=20?= =?UTF-8?q?test=20isolation=20v2=20=E2=80=94=20UPDATE=20not=20DELETE=20(FK?= =?UTF-8?q?-safe)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second CI attempt of the OnConflict test still failed, but with a different error this time: pre-seed: delete existing platform-agent rows: pq: update or delete on table "workspaces" violates foreign key constraint "workspaces_parent_id_fkey" on table "workspaces" The prior fix (DELETE all kind='platform' AND parent_id IS NULL) ran into the FK constraint because child workspaces can have parent_id pointing at the platform-agent row. The unique constraint allows at most one such row, but FK prevents deleting it while children reference it. The right shape: UPDATE (downgrade) instead of DELETE. This is exactly what installPlatformAgent itself does for prior roots (its step 0: `UPDATE workspaces SET kind = 'workspace' ... WHERE kind = 'platform' AND parent_id IS NULL AND id <> $1`). UPDATE is FK-safe (the children still have a valid parent_id, just pointing at a row that's no longer kind='platform') and clears the uniq-platform-root slot. This commit: - Replaces the pre-seed DELETE with an UPDATE. - Replaces the post-test DELETE in cleanup() with the same UPDATE (belt-and-suspenders: leave the slot in a state where the next test can seed cleanly). - The DELETE of OUR specific row by id still works (we created it with parent_id NULL, so nothing references it, and the UPDATE-kind change doesn't affect DELETE semantics on it). Refs: molecule-core#2615, core#2496, PR #2629 Co-Authored-By: Claude --- .../platform_agent_integration_test.go | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/workspace-server/internal/handlers/platform_agent_integration_test.go b/workspace-server/internal/handlers/platform_agent_integration_test.go index a6e1f5802..c83bf0ed8 100644 --- a/workspace-server/internal/handlers/platform_agent_integration_test.go +++ b/workspace-server/internal/handlers/platform_agent_integration_test.go @@ -323,35 +323,38 @@ func TestIntegration_PlatformAgentInstall_OnConflictUpdatesRuntime(t *testing.T) staleRuntime := "codex" // deliberately non-canonical // The schema has `uniq_workspaces_one_platform_root` (UNIQUE - // constraint allowing at most one row with kind='platform' AND + // partial index allowing at most one row with kind='platform' AND // parent_id IS NULL per database). A prior sibling test in this // package (e.g. TestIntegration_PlatformAgentInstall_ReparentsRoot // AndMovesAnchors) leaves a platform-agent row behind; its cleanup // deletes by its OWN uuid+name, so by the time this test runs, - // the slot may or may not be free depending on test scheduling and - // whether the prior cleanup ran. + // the slot may be occupied by a prior test's row (depending on + // test scheduling). // - // To make this test robust to test ordering and to the - // existing-test-row state, the pre-seed cleanup nukes ANY existing - // platform-agent row (any uuid, any name) — the slot is - // single-tenant by design, so this is safe in a fresh CI database - // (this is the only writer of kind='platform' rows; sub-tests' - // cleanup removes them at the end of their own run). The - // post-test cleanup then removes our specific rows. + // To make this test robust to test ordering, the pre-seed + // cleanup DOWNGRADES any existing platform root to 'workspace' + // (matching what installPlatformAgent itself does for prior + // roots — see its step 0). We use UPDATE not DELETE because + // child workspaces may have `parent_id` pointing at the platform + // root, and a DELETE would trip `workspaces_parent_id_fkey`. + // Downgrading (UPDATE kind='workspace') is FK-safe and clears + // the uniq-platform-root slot. cleanup := func() { _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE name = $1`, paName) _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE id = $1`, platformID) - // Belt-and-suspenders: also drop any other platform-agent row - // left by a prior test that somehow didn't clean up, so the - // next test's seed has a free slot. - _, _ = conn.ExecContext(ctx, `DELETE FROM workspaces WHERE kind = 'platform' AND parent_id IS NULL AND id <> $1`, platformID) + // Belt-and-suspenders: also downgrade any other platform-agent + // row left by a sibling test, so the next test's seed slot + // is free. + _, _ = conn.ExecContext(ctx, `UPDATE workspaces SET kind = 'workspace' WHERE kind = 'platform' AND parent_id IS NULL AND id <> $1`, platformID) } t.Cleanup(cleanup) - // Pre-seed: clear the slot completely (nuke any pre-existing - // platform-agent row, including the unique-constraint violating one - // from a prior sibling test). - if _, err := conn.ExecContext(ctx, `DELETE FROM workspaces WHERE kind = 'platform' AND parent_id IS NULL`); err != nil { - t.Fatalf("pre-seed: delete existing platform-agent rows: %v", err) + // Pre-seed: clear the uniq-platform-root slot by downgrading + // any existing platform root to 'workspace'. This is FK-safe + // and matches installPlatformAgent's own step 0 (so we're + // exercising the same shape the production code uses). + if _, err := conn.ExecContext(ctx, + `UPDATE workspaces SET kind = 'workspace' WHERE kind = 'platform' AND parent_id IS NULL`); err != nil { + t.Fatalf("pre-seed: downgrade existing platform-agent rows: %v", err) } cleanup() -- 2.52.0