Skip to content

fix(app-core): stabilize embed auth tests#10660

Merged
lalalune merged 4 commits into
developfrom
fix/9947-embed-postmerge-auth-tests
Jul 1, 2026
Merged

fix(app-core): stabilize embed auth tests#10660
lalalune merged 4 commits into
developfrom
fix/9947-embed-postmerge-auth-tests

Conversation

@NubsCarson

Copy link
Copy Markdown
Member

Summary

  • removes order-dependent global @elizaos/core mocking from embed handshake tests
  • adds an optional EmbedLaunchDeps hook so tests can inject hasRoleAccess while production keeps the real core role check
  • keeps the embed launch authorization seam fail-closed and unchanged for normal callers

Root cause

After #10638/#10643/#10653 landed on develop, the combined app-core embed/auth test run failed under the shared Vitest module cache. The handshake tests mocked @elizaos/core globally, but other files could import the real module first, so accepted OWNER/ADMIN cases failed as insufficient_role.

Validation

  • bun run build:core
  • bun run --cwd packages/app-core test src/api/embed-auth-routes.test.ts src/api/auth/embed-session-auth.test.ts src/api/auth/embed-session-token.test.ts src/api/auth/embed-handshake.test.ts src/api/auth/discord-exchange.test.ts src/api/auth/embed-end-to-end.test.ts
  • bun run --cwd packages/app test src/embed-bootstrap.test.ts
  • bun run --cwd plugins/plugin-discord test __tests__/slash-commands.test.ts
  • bun run --cwd plugins/plugin-telegram test src/messageManager.embed-launch.test.ts
  • bun run --cwd packages/app-core build
  • bunx biome check --write packages/app-core/src/api/auth/embed-handshake.ts packages/app-core/src/api/auth/embed-handshake.test.ts packages/app-core/src/api/auth/embed-end-to-end.test.ts

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: be7309ae-f61d-4a62-9c99-f884983d890d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/9947-embed-postmerge-auth-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lalalune

lalalune commented Jul 1, 2026

Copy link
Copy Markdown
Member

Reviewed #10660 locally at 8f8f16324f30. The dependency-injection seam is a good replacement for the previous global @elizaos/core mock and keeps production behavior on the real hasRoleAccess path. I do not see a code-level blocker.

Validation run:

  • bun run --cwd packages/app-core test src/api/embed-auth-routes.test.ts src/api/auth/embed-session-auth.test.ts src/api/auth/embed-session-token.test.ts src/api/auth/embed-handshake.test.ts src/api/auth/discord-exchange.test.ts src/api/auth/embed-end-to-end.test.ts: passed, 6 files / 60 tests.
  • bun run --cwd packages/app test src/embed-bootstrap.test.ts: passed, 13 tests.
  • bun run --cwd plugins/plugin-discord test __tests__/slash-commands.test.ts: passed, 5 tests.
  • bun run --cwd plugins/plugin-telegram test src/messageManager.embed-launch.test.ts: passed, 5 tests.
  • bun run --cwd packages/app-core typecheck: passed.
  • bunx biome check packages/app-core/src/api/auth/embed-handshake.ts packages/app-core/src/api/auth/embed-handshake.test.ts packages/app-core/src/api/auth/embed-end-to-end.test.ts: passed.
  • git diff --check origin/develop...HEAD: passed.

No merge from me because this PR is still draft and required GitHub checks are queued.

@NubsCarson NubsCarson marked this pull request as ready for review July 1, 2026 04:19

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@NubsCarson

Copy link
Copy Markdown
Member Author

Updated #10660 onto latest origin/develop (d4262967ef, includes #10647/#10661) with merge commit 0ac332a46c; no conflicts and diff remains limited to the three intended app-core auth/test files.

Current-base validation after merge:

  • git diff --check origin/develop...HEAD
  • bun run --cwd packages/app-core test src/api/embed-auth-routes.test.ts src/api/auth/embed-session-auth.test.ts src/api/auth/embed-session-token.test.ts src/api/auth/embed-handshake.test.ts src/api/auth/discord-exchange.test.ts src/api/auth/embed-end-to-end.test.ts (60 passed)
  • bun run --cwd packages/app test src/embed-bootstrap.test.ts (13 passed)
  • bun run --cwd plugins/plugin-discord test __tests__/slash-commands.test.ts (5 passed)
  • bun run --cwd plugins/plugin-telegram test src/messageManager.embed-launch.test.ts (5 passed)

Pushed current-base branch so GitHub checks restart without the stale cancelled contexts from the earlier ready-for-review transition.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@NubsCarson

Copy link
Copy Markdown
Member Author

Refreshed onto current origin/develop (a3d78672c6) after #10663/#10664/#10665/#10666 landed. New head: 7ab1c859149a.

Local validation on the refreshed branch:

  • bun run --cwd packages/app-core test src/api/embed-auth-routes.test.ts src/api/auth/embed-session-auth.test.ts src/api/auth/embed-session-token.test.ts src/api/auth/embed-handshake.test.ts src/api/auth/discord-exchange.test.ts src/api/auth/embed-end-to-end.test.ts -> 6 files / 60 tests passed.
  • bun run --cwd packages/app test src/embed-bootstrap.test.ts -> 14 tests passed.
  • bun run --cwd plugins/plugin-discord test __tests__/slash-commands.test.ts -> 5 tests passed.
  • bun run --cwd plugins/plugin-telegram test src/messageManager.embed-launch.test.ts -> 5 tests passed. punycode deprecation warning only.
  • bunx @biomejs/biome check packages/app-core/src/api/auth/embed-handshake.ts packages/app-core/src/api/auth/embed-handshake.test.ts packages/app-core/src/api/auth/embed-end-to-end.test.ts -> pass.
  • bun run --cwd packages/app-core build -> pass.
  • git diff --check origin/develop...HEAD -> pass.

Diff against latest develop remains scoped to the intended three app-core embed-auth files. GitHub checks are queued/UNSTABLE at the moment; I do not see a local blocker.

@lalalune

lalalune commented Jul 1, 2026

Copy link
Copy Markdown
Member

Reviewed current head 7ab1c859149a7eaf694d42fd395e9eeaacea84ea.

This looks scoped correctly: production still defaults to the core hasRoleAccess, while the tests inject a local role checker through EmbedLaunchDeps, avoiding the broad module mock that was destabilizing post-merge auth tests.

Validation run in an isolated worktree:

  • bun install --frozen-lockfile --ignore-scripts
  • bun run build:core — passed 64/64 tasks on the prior head before the final develop merge; @elizaos/app-core:build completed successfully in that run.
  • After updating to the current head 7ab1c859: bun test packages/app-core/src/api/auth/embed-handshake.test.ts packages/app-core/src/api/auth/embed-end-to-end.test.ts — 11 pass, 0 fail, 28 assertions.
  • bunx @biomejs/biome check packages/app-core/src/api/auth/embed-handshake.ts packages/app-core/src/api/auth/embed-handshake.test.ts packages/app-core/src/api/auth/embed-end-to-end.test.ts — passed.
  • git diff --check origin/develop...HEAD — passed.

No code changes from me on this PR. Current GitHub checks are still queued/pending, so I’m leaving merge to the normal green-check path.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@NubsCarson

Copy link
Copy Markdown
Member Author

Refreshed again onto current origin/develop (0a3fd649de) after #10667/#10641 landed. New head: 0e855d2866.

Local validation on the refreshed branch:

  • bun run --cwd packages/app-core test src/api/embed-auth-routes.test.ts src/api/auth/embed-session-auth.test.ts src/api/auth/embed-session-token.test.ts src/api/auth/embed-handshake.test.ts src/api/auth/discord-exchange.test.ts src/api/auth/embed-end-to-end.test.ts -> 6 files / 60 tests passed.
  • bun run --cwd packages/app test src/embed-bootstrap.test.ts -> 14 tests passed.
  • bun run --cwd plugins/plugin-discord test __tests__/slash-commands.test.ts -> 5 tests passed.
  • bun run --cwd plugins/plugin-telegram test src/messageManager.embed-launch.test.ts -> 5 tests passed (punycode deprecation warning only).
  • bunx @biomejs/biome check packages/app-core/src/api/auth/embed-handshake.ts packages/app-core/src/api/auth/embed-handshake.test.ts packages/app-core/src/api/auth/embed-end-to-end.test.ts -> pass.
  • bun run --cwd packages/app-core build -> pass.
  • git diff --check origin/develop...HEAD -> pass.

Diff against latest develop is still scoped to the intended three app-core embed-auth files. No local blocker found.

@lalalune lalalune merged commit 617621c into develop Jul 1, 2026
11 of 55 checks passed
@lalalune lalalune deleted the fix/9947-embed-postmerge-auth-tests branch July 1, 2026 04:59
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.

2 participants