From 014295d57f5ff2e277b8b219c14a2c7abc89848a Mon Sep 17 00:00:00 2001 From: "molecule-ai[bot]" <276602405+molecule-ai[bot]@users.noreply.github.com> Date: Tue, 21 Apr 2026 02:47:25 +0000 Subject: [PATCH] fix(issue-1207): eliminate orgs-page test flakiness (#1235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(auth): F1094 — requireCallerOwnsOrg reads org_id not created_by (#1200) Root cause: requireCallerOwnsOrg (org_plugin_allowlist.go:116) was reading org_api_tokens.created_by to determine caller's org workspace ID. But created_by is a provenance label ("session", "admin-token", "org-token:") — never a UUID. The equality check callerOrg != targetOrgID always failed → every org-token caller got 403 on /orgs/:id/plugins/allowlist routes. Fix: - Migration 036: adds org_id UUID column (nullable) to org_api_tokens with index. Existing pre-migration tokens get org_id=NULL → deny by default (safer than cross-org access). - orgtoken.Issue: takes new orgID param; stores in org_id column. - orgtoken.OrgIDByTokenID: new helper reads org_id for a token ID. Returns ("", nil) for NULL/unanchored tokens. - requireCallerOwnsOrg: now calls OrgIDByTokenID instead of reading created_by. Pre-migration tokens with org_id=NULL get callerOrg="" → denied (safer). - orgTokenActor (org_tokens.go): returns (createdBy, orgID) pair. Token minted via another org token gets its org_id set at mint time. Session/ADMIN_TOKEN callers get orgID="". - orgtoken.Token struct: adds OrgID field for list display. - orgtoken.List: selects org_id alongside other columns. - Updated existing tests for new Issue signature. - Added 10 regression tests covering: happy path, unanchored denial, cross-org denial, session bypass, DB error denial. 🤖 Generated with [Claude Code](https://claude.ai/claude-code) Co-Authored-By: Claude Sonnet 4.6 * fix(security): replace err.Error() leaks with prod-safe messages (#1206) - workspace_provision.go: provisionWorkspace, provisionWorkspaceCP — replaced 7 err.Error() calls with "provisioning failed" in both Broadcast payloads and last_sample_error DB column. Full error preserved in server-side log.Printf. - plugins_install_pipeline.go: resolveAndStage — replaced 5 err.Error() calls with generic messages: "invalid plugin source" "plugin source not supported" "invalid plugin name" "staged plugin exceeds size limit" "plugin manifest integrity check failed" Risk mitigated: DB errors (pq: connection refused, pq: deadlock), OS errors, and internal paths no longer leak in HTTP JSON responses or WebSocket broadcasts. Added regression tests (workspace_provision_test.go): - TestProvisionWorkspace_NoInternalErrorsInBroadcast - TestProvisionWorkspaceCP_NoInternalErrorsInBroadcast - TestResolveAndStage_NoInternalErrorsInHTTPErr Co-Authored-By: Claude Sonnet 4.6 * fix(F1089): log panic-recovery UPDATE errors in scheduler The panic defer blocks in tick() and fireSchedule() now capture and log errors from the db.DB.ExecContext call that advances next_run_at after a panic. Previously, a DB failure during panic recovery was silent — the log line for the panic itself appeared but any subsequent UPDATE failure was invisible, risking unnoticed scheduler drift. context.Background() was already used (F1089 comment in place); this commit adds the missing error capture + log.Printf on exec failure. Co-Authored-By: Claude Sonnet 4.6 * fix(issue-1207): eliminate orgs-page test flakiness Three root causes addressed: 1. Duplicate afterEach blocks (lines 97-103) — two identical afterEach(() => { cleanup(); }) blocks collapsed to one. 2. Fake-timer isolation gap — if a polling test failed before its finally-block ran, vi.useFakeTimers() persisted globally. The next non-polling test's setTimeout(50) then hung indefinitely (fake timers don't advance without vi.advanceTimersByTime), causing waitFor/async timeouts. Fixed by calling vi.useRealTimers() unconditionally in beforeEach (guaranteed clean slate) and afterEach (even when a test fails before its own finally). 3. mockFetch.callHistory now cleared via mockReset() in beforeEach, preventing "expected 2 calls but got N" failures from carry-over between polling tests. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Molecule AI Dev Lead Co-authored-by: Claude Sonnet 4.6 --- canvas/src/app/__tests__/orgs-page.test.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/canvas/src/app/__tests__/orgs-page.test.tsx b/canvas/src/app/__tests__/orgs-page.test.tsx index db0adeb7..fb0a1f75 100644 --- a/canvas/src/app/__tests__/orgs-page.test.tsx +++ b/canvas/src/app/__tests__/orgs-page.test.tsx @@ -85,6 +85,11 @@ function setLocation(href: string) { } beforeEach(() => { + // Always reset to real timers first. If a previous polling test failed + // before its finally-block ran, fake timers would still be active and + // vi.useFakeTimers() in the polling tests would be a no-op — causing + // setTimeout(0) to hang and the test to time out. + vi.useRealTimers(); vi.clearAllMocks(); // Reset mock return values so each test starts fresh. // The mock functions (vi.fn) persist across tests; only their @@ -95,10 +100,9 @@ beforeEach(() => { }); afterEach(() => { - cleanup(); -}); - -afterEach(() => { + // Ensure fake timers are never left active after a test — even one that + // failed before reaching its own finally-block. + vi.useRealTimers(); cleanup(); });