Skip to content

[Fix] Credential tests fail when shared grants overlap - #3610

Open
roomote-roomote[bot] wants to merge 1 commit into
developfrom
fix/isolate-credential-db-fixtures-0fh26y2vk7gm0
Open

roomote-roomote[bot] wants to merge 1 commit into
developfrom
fix/isolate-credential-db-fixtures-0fh26y2vk7gm0

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

​Opened on behalf of @daniel-lxs. Follow up by mentioning @roomote-roomote, in the web UI, or in Telegram.

Related issue

Internal CI reliability follow-up; no separate issue.

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

What changed

Run the two credential fixture suites in the SDK's existing serial global-database-state project. Their deployment-visible test grants intentionally appear to other actors, so running both files in parallel contaminates valid assertions about visible grants and substitutes.

Only two existing file memberships and an explanatory comment change. Every assertion, fixture and production credential policy stays intact; ordinary SDK suites retain their parallel project. This trades a small amount of fixture concurrency for reliable checks, without introducing a new runner or timing budget.

How it was tested

  • Current develop 71ee50ca: identical real-PostgreSQL Vitest command selecting both files with --maxWorkers=2 failed all three runs, with 1, 4 and 1 failures out of 42 cases. The failures contained another file's deliberately deployment-visible grant.
  • Exact head b4e830d46fc0a9f95a3aa12bfe5627e9c26da08c: the identical command passed 42/42 on all three runs. Verbose adjacent execution confirms both files use sdk-global-db-state after the ordinary project, rather than merely hiding failures.
  • A 62-case run with artifact-publication and real independent-process draft-intent persistence passed. A direct query against the actual test database confirmed zero remaining credential/approval fixture rows afterward.
  • Affected SDK fast types, formatting, whitespace and normal commit/push hooks passed. All applicable exact-head hosted gates pass/skip, with a clean exact-head review, zero threads and mergeability rechecked. Browser proof is not applicable; no production behavior changes.
  • The entire SDK suite also passes in the isolated six-diff compatibility checkout: 198 files/2,662 tests with two local workers. This is separate compatibility evidence, not a claim that an earlier cold full-root attempt was green.
  • Broader compatibility probing separately exposed an unchanged cold dynamic-import timeout and a fixture assuming its checkout is outside /tmp; neither is changed or claimed repaired here. This PR fixes only the independently reproduced cross-file credential contamination.

Reproduction:

pnpm exec dotenvx run --quiet -f .env.test -- pnpm --filter @roomote/sdk exec vitest run src/server/lib/__tests__/service-credentials.test.ts src/server/lib/__tests__/credential-egress.integration.test.ts --maxWorkers=2

Related PRs

Independent work from the same shift: shared-check cache correctness, foreground deadlines, review debounce, Users recovery, native development watcher.

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally (normal broad fast equivalents and affected direct typecheck passed)
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset (test configuration only)

@roomote-community

roomote-community Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

No code issues found. See session

Reviewed the project inclusion/exclusion rules, serial scheduling, and both credential suites’ shared-grant assertions and cleanup. Both files remain included in sdk-global-db-state and are excluded from the parallel sdk project. Current-head CI Test, Lint, and Type Check checks passed; Docker build and JavaScript/TypeScript CodeQL analysis were still running when checked.

Reviewed b4e830d

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant