Repository navigation
Remove page-to-markdown capability - #192
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThis PR removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
kentcdodds
marked this pull request as ready for review
April 17, 2026 23:56
Contributor
|
🔎 Preview deployed: https://kody-pr-192.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/worker/src/mcp/tools/search.ts (1)
162-162: Nit: stray leading space before the bullet.Line 162 begins with
- \...`(a leading space), while the surrounding bullets on lines 159–161 and 163–166 use- ` with no leading space. Likely a leftover from the edit — worth normalizing for consistency.Proposed fix
- - \`{ "entity": "kody_official_guide:capability" }\` +- \`{ "entity": "kody_official_guide:capability" }\`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/tools/search.ts` at line 162, Normalize the inconsistent bullet by removing the stray leading space so the line that currently reads ` - `{ "entity": "kody_official_guide:capability" }`` becomes `- `{ "entity": "kody_official_guide:capability" }``; edit the markdown in the search.ts snippet (the bullet list around the existing entries) to ensure all bullets use the same `- ` prefix formatting.docs/contributing/environment-variables.md (1)
99-108: Minor:CLOUDFLARE_API_TOKENdescription reads as single-purpose now.Line 99-100 says the token is used by the internal API client "for the Cloudflare Email sender." Given the internal API client is also used for other Cloudflare API calls (per the
cloudflare-rest-client.tsand the/client/v4/*note at Line 33 of.env.example), consider rewording to "…used by the internal API client (e.g., for the Cloudflare Email sender)" so readers don't infer email is the only use. Low priority.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributing/environment-variables.md` around lines 99 - 108, The CLOUDFLARE_API_TOKEN doc line is phrased as if it's only for the Email sender; update the description to indicate it's used by the internal Cloudflare API client more generally (e.g., "used by the internal API client (e.g., for the Cloudflare Email sender)") so readers know it covers other Cloudflare REST calls; adjust the text near CLOUDFLARE_API_TOKEN in environment-variables.md and ensure consistency with the note in .env.example and the cloudflare-rest-client.ts mention.docs/contributing/skill-patterns/cloudflare-developer-docs.md (1)
71-78: Consider documenting HTML fallback behavior for readers.The previous version used
content-typeto detect HTML and invoked a markdown fallback; now the skill returns whatever the docs site responds with. If a path returns HTML (e.g., a page without a markdown variant), the caller will get raw HTML inbody. Worth a one-liner noting that the caller should checkcontentTypeand that HTML responses won't be auto-converted anymore. Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributing/skill-patterns/cloudflare-developer-docs.md` around lines 71 - 78, Add a one-line note in the docs near the fetch response example explaining that the function returns the raw body and callers must check the returned contentType (the `contentType` field) before treating `body` as Markdown because HTML responses (e.g., from a page without a markdown variant) are not auto-converted anymore; reference the `markdownTokenEstimate` and `body` slicing behavior so callers know to inspect `contentType` and handle HTML fallback themselves.docs/contributing/setup-manifest.md (1)
176-180: DuplicateCLOUDFLARE_API_TOKENentry.
CLOUDFLARE_API_TOKENis already documented earlier in the GitHub Actions list at Line 107-108. This second entry at Line 176-180 (under "How to get/set each value") is the "how to get it" counterpart for the earlier bullet, which is the established pattern in this doc — so the duplication is intentional. Just flagging to confirm the wording here ("required for remote AI; user API calls use account secrets + skills") stays in sync with the earlier bullet's framing, which talks about Workers deploy + D1 edit access. The two descriptions emphasize different uses of the same token and could confuse readers scanning only one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributing/setup-manifest.md` around lines 176 - 180, The duplicate CLOUDFLARE_API_TOKEN entries should be reconciled so their wording is consistent and clearly distinguishes the two uses: update the earlier GitHub Actions bullet and the later "How to get/set each value" paragraph (search for the token name CLOUDFLARE_API_TOKEN) to use matching phrasing that mentions both Workers deploy + D1 edit access and remote AI/user API calls via account secrets + skills, or add a short clarifier sentence in one of the two locations that cross-references the other to avoid confusion for readers who only see one section.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/contributing/environment-variables.md`:
- Around line 99-108: The CLOUDFLARE_API_TOKEN doc line is phrased as if it's
only for the Email sender; update the description to indicate it's used by the
internal Cloudflare API client more generally (e.g., "used by the internal API
client (e.g., for the Cloudflare Email sender)") so readers know it covers other
Cloudflare REST calls; adjust the text near CLOUDFLARE_API_TOKEN in
environment-variables.md and ensure consistency with the note in .env.example
and the cloudflare-rest-client.ts mention.
In `@docs/contributing/setup-manifest.md`:
- Around line 176-180: The duplicate CLOUDFLARE_API_TOKEN entries should be
reconciled so their wording is consistent and clearly distinguishes the two
uses: update the earlier GitHub Actions bullet and the later "How to get/set
each value" paragraph (search for the token name CLOUDFLARE_API_TOKEN) to use
matching phrasing that mentions both Workers deploy + D1 edit access and remote
AI/user API calls via account secrets + skills, or add a short clarifier
sentence in one of the two locations that cross-references the other to avoid
confusion for readers who only see one section.
In `@docs/contributing/skill-patterns/cloudflare-developer-docs.md`:
- Around line 71-78: Add a one-line note in the docs near the fetch response
example explaining that the function returns the raw body and callers must check
the returned contentType (the `contentType` field) before treating `body` as
Markdown because HTML responses (e.g., from a page without a markdown variant)
are not auto-converted anymore; reference the `markdownTokenEstimate` and `body`
slicing behavior so callers know to inspect `contentType` and handle HTML
fallback themselves.
In `@packages/worker/src/mcp/tools/search.ts`:
- Line 162: Normalize the inconsistent bullet by removing the stray leading
space so the line that currently reads ` - `{ "entity":
"kody_official_guide:capability" }`` becomes `- `{ "entity":
"kody_official_guide:capability" }``; edit the markdown in the search.ts snippet
(the bullet list around the existing entries) to ensure all bullets use the same
`- ` prefix formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5cb401c-fff1-4481-993e-edefdf2163a2
📒 Files selected for processing (22)
docs/contributing/environment-variables.mddocs/contributing/mock-api-servers.mddocs/contributing/setup-manifest.mddocs/contributing/setup.mddocs/contributing/skill-patterns/cloudflare-api-v4.mddocs/contributing/skill-patterns/cloudflare-developer-docs.mddocs/contributing/skill-patterns/index.mddocs/use/search.mdpackages/mock-servers/cloudflare/src/worker.tspackages/worker/.env.examplepackages/worker/src/mcp/capabilities/coding/coding-capabilities.node.test.tspackages/worker/src/mcp/capabilities/coding/domain.tspackages/worker/src/mcp/capabilities/coding/fetch-page-markdown.tspackages/worker/src/mcp/capabilities/coding/index.tspackages/worker/src/mcp/capabilities/coding/page-to-markdown.tspackages/worker/src/mcp/cloudflare/cloudflare-rest-client.tspackages/worker/src/mcp/server-instructions.tspackages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.tspackages/worker/src/mcp/tools/execute.node.test.tspackages/worker/src/mcp/tools/execute.tspackages/worker/src/mcp/tools/search-format.tspackages/worker/src/mcp/tools/search.ts
💤 Files with no reviewable changes (6)
- docs/contributing/skill-patterns/cloudflare-api-v4.md
- packages/worker/src/mcp/capabilities/coding/index.ts
- packages/mock-servers/cloudflare/src/worker.ts
- packages/worker/src/mcp/capabilities/coding/page-to-markdown.ts
- packages/worker/src/mcp/capabilities/coding/coding-capabilities.node.test.ts
- packages/worker/src/mcp/capabilities/coding/fetch-page-markdown.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/contributing/setup-manifest.md (1)
178-182: Consolidate duplicateCLOUDFLARE_API_TOKENsetup entries.Line 178 through Line 182 repeats guidance already introduced at Line 137 onward. Consider merging these into a single subsection so future permission updates happen in one place.
Suggested doc cleanup
-- `CLOUDFLARE_API_TOKEN` (required for remote AI and for Cloudflare API - workflows that run with account secrets + skills) - - This is the same token described earlier for Workers deploy + D1 access. - When you reuse it for remote AI or Cloudflare API workflows, also include - the product permissions needed for those APIs. + - Include additional product permissions needed for remote AI and Cloudflare + API workflows when reusing this token.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributing/setup-manifest.md` around lines 178 - 182, Duplicate guidance about CLOUDFLARE_API_TOKEN exists in the doc; consolidate into a single subsection named for CLOUDFLARE_API_TOKEN, remove the repeated block, and merge any unique permission details so updates only need to be made in one place; ensure the final subsection includes the note that this token is the same one used for Workers deploy + D1 access and that additional product permissions must be added when reusing it for remote AI or Cloudflare API workflows (keep the original wording from the earlier occurrence and delete the later duplicate).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/contributing/setup-manifest.md`:
- Around line 178-182: Duplicate guidance about CLOUDFLARE_API_TOKEN exists in
the doc; consolidate into a single subsection named for CLOUDFLARE_API_TOKEN,
remove the repeated block, and merge any unique permission details so updates
only need to be made in one place; ensure the final subsection includes the note
that this token is the same one used for Workers deploy + D1 access and that
additional product permissions must be added when reusing it for remote AI or
Cloudflare API workflows (keep the original wording from the earlier occurrence
and delete the later duplicate).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2135735c-eb5e-4a3b-a2af-d914ff6dd8cf
📒 Files selected for processing (4)
docs/contributing/environment-variables.mddocs/contributing/setup-manifest.mddocs/contributing/skill-patterns/cloudflare-developer-docs.mdpackages/worker/src/mcp/tools/search.ts
✅ Files skipped from review due to trivial changes (1)
- packages/worker/src/mcp/tools/search.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/contributing/environment-variables.md
- docs/contributing/skill-patterns/cloudflare-developer-docs.md
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
page_to_markdowncoding capability and its Browser Rendering helper implementationTesting
npm test -- --run packages/worker/src/mcp/capabilities/coding/coding-capabilities.node.test.ts packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.ts packages/worker/src/mcp/tools/execute.node.test.tsnpm test -- --run packages/worker/src/mcp/tools/execute.node.test.ts packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.tsnpm run typecheckSummary by CodeRabbit
Removed Features
Documentation