Skip to content

test(worker): cover Entra token acquisition after the msal-node v7 bump - #157

Merged
semics-tech merged 2 commits into
mainfrom
agent/msal-v7-tests
Oct 30, 2026
Merged

semics-tech merged 2 commits into
mainfrom
agent/msal-v7-tests

Conversation

@semics-tech

@semics-tech semics-tech commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

Why

#151 forced @azure/msal-node to ^7.1.0 through a pnpm override (@azure/identity>@azure/msal-node). The Entra path had no tests.

Finding: no source change needed

  • The repo never imports msal-node. The only use is acquireEntraToken in packages/worker/src/credentials.ts, which calls @azure/identity's DefaultAzureCredential().getToken(scope).
  • @azure/identity@4.14.0 declares @azure/msal-node: ^6.0.0, so the override takes it outside its declared range. This is the real risk, and it is not ours to fix in our own code.
  • msal-node v7.1.1 ships no changelog or migration notes, so the API was checked directly. Every msal-node symbol identity uses (ManagedIdentityApplication, ConfidentialClientApplication, PublicClientApplication, AuthError, LogLevel, INativeBrokerPlugin, ICachePlugin) exists in v7. Identity's index.js loads under v7 and ManagedIdentityApplication constructs.
  • Not verified: a real token fetch from Azure. There is no managed identity in the sandbox, so v7 against a live IMDS or App Service endpoint is untested. Worth one manual check on an Azure VM before 10 Nov.

What changed

Only packages/worker/test/entra-credentials.test.ts (15 tests, @azure/identity mocked). No source changes.

Area Covered
Token acquisition returns the token; appends /.default to a bare audience and leaves an existing one alone; no options for system-assigned identity; clientId goes through as managedIdentityClientId
Cache and refresh each call builds a fresh credential, so a rotated token is picked up; a failure does not poison the next call
Error path construction failure becomes a CredentialError; null/empty token becomes a CredentialError naming the scope; getToken rejection propagates unchanged
buildCallMetadata authorization: Bearer …; forwards clientId; missing audience fails before identity is touched; failed acquisition surfaces

Observation, not changed

A new DefaultAzureCredential is built on every call, so msal's in-memory cache never survives. Correct but inefficient; caching the credential would be a small follow-up.

CI note

The earlier "Control plane image builds" failure was infra, not the repo. The image built fine and the job failed at the end exporting the GitHub Actions cache (error writing layer blob: not_found). The same check passes on #155. Main was merged into this branch (rebase would need a force-push) so CI reruns on a fresh commit.

🤖 Generated with Claude Code


Draft opened by Oli's repos agent.

Agent Harness and others added 2 commits October 28, 2026 07:09
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@semics-tech
semics-tech marked this pull request as ready for review October 29, 2026 07:06
@semics-tech
semics-tech merged commit 33ec093 into main Oct 30, 2026
14 checks passed
@semics-tech
semics-tech deleted the agent/msal-v7-tests branch October 30, 2026 02:00
semics-tech added a commit that referenced this pull request Oct 31, 2026
…rotobuf (#155)

Supersedes
[#152](#152). Same
14 dependency updates, plus one commit regenerating
`packages/protocol/src/gen` for ts-proto 2.18.3, which fixes the failing
"Check generated protobuf is current" step.

Update 30 Oct: merged main (incl.
[#157](#157)) into
the branch, no rebase. Conflicts were in the root `package.json`,
`packages/dashboard/package.json` and `pnpm-lock.yaml`. I kept the
higher version of each (vitest ^4.1.11, vite ^8.3.5, typescript-eslint
^8.72.0), bumped `@vitest/coverage-v8` to ^4.1.11 to match, and
regenerated the lockfile with `pnpm install --lockfile-only`. ts-proto
stays at 2.18.3, so the generated protobuf is unchanged.

Local: typecheck passes and lint has 0 errors (1 existing warning).
`proto:check` couldn't run in my sandbox because tsx can't open its IPC
socket (EPERM), so CI is the judge.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Agent Harness <agent@SEMICSs-Mac-mini.local>
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
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