Skip to content

fix(mcp): do not let empty cdpHeaders from env override config file#39866

Merged
dgozman merged 2 commits intomicrosoft:mainfrom
harrytran001:fix(mcp)-do-not-let-empty-cdpHeaders-from-env-override-config-file
Mar 27, 2026
Merged

fix(mcp): do not let empty cdpHeaders from env override config file#39866
dgozman merged 2 commits intomicrosoft:mainfrom
harrytran001:fix(mcp)-do-not-let-empty-cdpHeaders-from-env-override-config-file

Conversation

@harrytran001
Copy link
Copy Markdown
Contributor

Summary

  • Preserve browser.cdpHeaders from config files when PLAYWRIGHT_MCP_CDP_HEADERS is unset.
  • Avoid producing/merging empty cdpHeaders overrides from env parsing.
  • Add regression test in tests/mcp/config.spec.ts.

@harrytran001
Copy link
Copy Markdown
Contributor Author

@harrytran001 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

contextOptions,
cdpEndpoint: cliOptions.cdpEndpoint,
cdpHeaders: cliOptions.cdpHeader,
cdpHeaders: nonEmptyRecord(cliOptions.cdpHeader),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems like a better solution could be replacing return previous || {}; with return previous; in the headerParser() function. Basically, returning undefined for the empty headers string. Then, usual logic with filtering undefined would just work. WDYT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree. I made the change.

@harrytran001 harrytran001 requested a review from dgozman March 27, 2026 00:23
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

5562 passed, 340 skipped


Merge workflow run.

Copy link
Copy Markdown
Collaborator

@dgozman dgozman left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@dgozman dgozman merged commit 6165ad5 into microsoft:main Mar 27, 2026
11 checks passed
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