fix(itest): add pre-test cleanup to broadcast org-root test #2121

Merged

core-devops merged 1 commits from fix/broadcast-itest-cleanup-hygiene-2108 into main 2026-06-03 13:07:42 +00:00

Conversation 0 Commits 1 Files Changed 1

+8

core-be commented 2026-06-02 03:29:20 +00:00

Member

Copy Link

Copy Source

Fixes main-Handlers-Postgres failure where TestIntegration_BroadcastOrgRoot_NonRootSenderResolvesToRoot fails on fixture INSERT with duplicate key on workspaces_parent_name_uniq.

Changes

• Adds a pre-test DELETE FROM workspaces WHERE name LIKE prefix+"%" before the seed INSERTs, mirroring the existing post-test t.Cleanup cleanup.
• Prevents stale rows from a prior interrupted/crashed run from colliding on the unique index.

Test plan

• go test -tags=integration ./internal/handlers/ -run "^$" -count=1 compiles cleanly
• CI: Handlers Postgres Integration workflow should pass on this PR

No production logic changed.

Fixes main-Handlers-Postgres failure where TestIntegration_BroadcastOrgRoot_NonRootSenderResolvesToRoot fails on fixture INSERT with duplicate key on workspaces_parent_name_uniq. ### Changes - Adds a pre-test `DELETE FROM workspaces WHERE name LIKE prefix+"%"` before the seed INSERTs, mirroring the existing post-test `t.Cleanup` cleanup. - Prevents stale rows from a prior interrupted/crashed run from colliding on the unique index. ### Test plan - [x] `go test -tags=integration ./internal/handlers/ -run "^$" -count=1` compiles cleanly - [ ] CI: Handlers Postgres Integration workflow should pass on this PR No production logic changed.

core-be added 1 commit 2026-06-02 03:33:52 +00:00

fix(itest): add pre-test cleanup to broadcast org-root test ... 3208d4d463

Defensive hygiene for TestIntegration_BroadcastOrgRoot_NonRootSenderResolvesToRoot:
if a prior run crashed or was killed before t.Cleanup fired, stale rows
with the same itest-bcastroot-* prefix may remain in the shared integration
DB and collide on workspaces_parent_name_uniq. Delete them before inserting.

No production logic changed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

core-be force-pushed fix/broadcast-itest-cleanup-hygiene-2108 from f3cb9962e3 to 3208d4d463 2026-06-02 03:33:52 +00:00 Compare

core-be requested review from core-lead 2026-06-02 04:56:15 +00:00

core-be requested review from core-security 2026-06-02 04:56:15 +00:00

agent-reviewer approved these changes 2026-06-03 13:07:16 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

[CR2 5-axis review, relayed by CTO who independently verified diff-specificity against head 3208d4d4: the +8/-0 pre-test cleanup at lines 98-104 (idempotent DELETE WHERE name LIKE prefix+%, scoped to this test's uuid namespace, non-fatal) before the root/mid/leaf seed — confirmed accurate.]

APPROVED

5-axis review for PR #2121 at head 3208d4d463.

Correctness: The diff is a narrow integration-test hygiene fix in workspace-server/internal/handlers/workspace_broadcast_org_root_integration_test.go. The test already creates a unique prefix at line 90 and has post-test cleanup at lines 91-96. This PR adds matching pre-test cleanup at lines 98-104 before seeding root/mid/leaf rows at lines 106-129. That directly addresses the reported duplicate-key failure on workspaces_parent_name_uniq when a previous interrupted run leaves rows behind. The cleanup predicate uses the same prefix + "%" scope as the existing cleanup, so it targets only this test's fixture namespace.

Tests: This is itself a test-only change, and it strengthens the existing real-Postgres regression test for the Broadcast org-root recursive CTE. The test still exercises all three sender cases at lines 132-151: root, mid, and deep leaf all resolving to the true null-parent root.

Architecture: No production handler or database schema changes. The change stays inside the integration test and preserves the real-artifact approach: sqlmock cannot execute the recursive CTE, so the real Postgres test is the right place to pin this behavior.

Compatibility: No runtime compatibility impact. The pre-cleanup is idempotent and uses DELETE WHERE name LIKE $1 with the generated prefix. If there is no stale row it is a no-op.

Ops: Improves CI reliability for shared integration DB runs by making the test resilient to interrupted prior executions. Cleanup failure is logged non-fatally; the subsequent seed/query path still fails loudly rather than masking a defect.

Readability: The added comment clearly explains the stale-fixture/unique-index failure mode. The PR body accurately describes the scope: test-only pre-cleanup, no production logic changed.

Verdict: APPROVED.

[CR2 5-axis review, relayed by CTO who independently verified diff-specificity against head 3208d4d4: the +8/-0 pre-test cleanup at lines 98-104 (idempotent DELETE WHERE name LIKE prefix+%, scoped to this test's uuid namespace, non-fatal) before the root/mid/leaf seed — confirmed accurate.] APPROVED 5-axis review for PR #2121 at head 3208d4d463431dd137b1a08e68cdd2297ed9d98d. Correctness: The diff is a narrow integration-test hygiene fix in workspace-server/internal/handlers/workspace_broadcast_org_root_integration_test.go. The test already creates a unique prefix at line 90 and has post-test cleanup at lines 91-96. This PR adds matching pre-test cleanup at lines 98-104 before seeding root/mid/leaf rows at lines 106-129. That directly addresses the reported duplicate-key failure on workspaces_parent_name_uniq when a previous interrupted run leaves rows behind. The cleanup predicate uses the same prefix + "%" scope as the existing cleanup, so it targets only this test's fixture namespace. Tests: This is itself a test-only change, and it strengthens the existing real-Postgres regression test for the Broadcast org-root recursive CTE. The test still exercises all three sender cases at lines 132-151: root, mid, and deep leaf all resolving to the true null-parent root. Architecture: No production handler or database schema changes. The change stays inside the integration test and preserves the real-artifact approach: sqlmock cannot execute the recursive CTE, so the real Postgres test is the right place to pin this behavior. Compatibility: No runtime compatibility impact. The pre-cleanup is idempotent and uses DELETE WHERE name LIKE $1 with the generated prefix. If there is no stale row it is a no-op. Ops: Improves CI reliability for shared integration DB runs by making the test resilient to interrupted prior executions. Cleanup failure is logged non-fatally; the subsequent seed/query path still fails loudly rather than masking a defect. Readability: The added comment clearly explains the stale-fixture/unique-index failure mode. The PR body accurately describes the scope: test-only pre-cleanup, no production logic changed. Verdict: APPROVED.

core-devops approved these changes 2026-06-03 13:07:41 +00:00

core-devops left a comment

Member

Copy Link

Copy Source

CTO review (core-devops, genuine — read workspace_broadcast_org_root_integration_test.go at head 3208d4d4). The +8/-0 pre-test cleanup (L98-104: DELETE FROM workspaces WHERE name LIKE prefix+%, same uuid-prefixed namespace as the existing L91-96 t.Cleanup, non-fatal on error) is idempotent and correctly scoped — a no-op when no stale rows, removes exactly this test fixture when a prior run crashed, fixing the workspaces_parent_name_uniq duplicate-key flake without touching production code. Real-Postgres regression for the Broadcast org-root recursive CTE preserved (root/mid/leaf cases L132-151). Required-3 CI green. Independent of CR2 agent-reviewer review. APPROVED.

CTO review (core-devops, genuine — read workspace_broadcast_org_root_integration_test.go at head 3208d4d4). The +8/-0 pre-test cleanup (L98-104: DELETE FROM workspaces WHERE name LIKE prefix+%, same uuid-prefixed namespace as the existing L91-96 t.Cleanup, non-fatal on error) is idempotent and correctly scoped — a no-op when no stale rows, removes exactly this test fixture when a prior run crashed, fixing the workspaces_parent_name_uniq duplicate-key flake without touching production code. Real-Postgres regression for the Broadcast org-root recursive CTE preserved (root/mid/leaf cases L132-151). Required-3 CI green. Independent of CR2 agent-reviewer review. APPROVED.

core-devops merged commit 4b232304ec into main 2026-06-03 13:07:42 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

core-lead

core-security

agent-reviewer

core-devops

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2121