Repository navigation
Conversation
There was a problem hiding this comment.
The fix works. I ran the #59822 reproduction (a mock model calling webfetch on https://docs.x.ai/grok-bot/security.md) from source on both checkouts. On the base it fails with a 404. With this change it returns the page's markdown. Non-.md URLs behave as before. tool-webfetch.test.ts passes and tsc -b for packages/core is clean.
Public API: unchanged.
The new new URL(url) call is safe, because the requested URL and every redirect target are checked before a request is built. Redirects and the Cloudflare User-Agent retry go through request(), so they use the same Accept rule. One small gap: only .md is handled, not .markdown, which the issue also mentioned. That doesn't block this change.
There was a problem hiding this comment.
The new commit only merges dev into the branch. The webfetch change and its test are the same as in my last review, and the merge doesn't touch them. On the new head I re-ran the #59822 reproduction from source: https://docs.x.ai/grok-bot/security.md now returns its markdown. tool-webfetch.test.ts passes and tsc -b for packages/core is clean.
Public API: unchanged.
No new concerns.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
The new commits extend the check to .markdown, which closes the one gap from my first review. The rest of the new diff is a merge of dev. On the new head I re-ran the #59822 reproduction from source: https://docs.x.ai/grok-bot/security.md returns its markdown. tool-webfetch.test.ts passes and tsc -b for packages/core is clean.
Public API: unchanged.
No new concerns.
Issue for this PR
Closes #59822
Type of change
What does this PR do?
Some documentation hosts return 404 when
text/markdownis advertised for URLs ending in.mdor.markdown.webfetchnow sendsAccept: */*for either path suffix and keeps its existing negotiation for other URLs.How did you verify your code works?
npm exec --yes --package=bun@1.4.4 -- bun run script/test.ts test/tool-webfetch.test.ts(91 passed)npm exec --yes --package=bun@1.4.4 -- bun typecheckfrompackages/core(1,737 files across 2 projects)Visual
Screenshots / recordings
Not a UI change.
Checklist