-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix: improve image URL parsing robustness across providers #8015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
recheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Logic looks good but lets pull this out into a common util somewhere to prevent code duplication
162d49a to
7e9055d
Compare
Thank you! Refactored, and I think moving the url parsing logic to core/util/url.ts would be a good idea here, or let me know if you'd prefer somewhere else? |
|
|
7e9055d to
3436b3e
Compare
|
@dcs-soni can you merge |
|
I have read the CLA Document and I hereby sign the CLA |
|
@Patrick-Erichsen The prettier and CLA checks were failing which are fixed now. Still some unrelated checks are failing. |
|
I think the only one blocking here is the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
28 issues found across 234 files
Note: This PR contains a large number of files. cubic only reviews up to 150 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all 28 issues)
Understand the root cause of the following 28 issues and fix them.
<file name="docs/guides/notion-continue-guide.mdx">
<violation number="1" location="docs/guides/notion-continue-guide.mdx:115">
The section below still documents Continue CLI usage, so renaming this heading to “Continue AI Agent” misleads readers about which product or interface the instructions cover. Please keep the heading aligned with the CLI-focused content.</violation>
</file>
<file name=".github/workflows/runloop-blueprint-template.json">
<violation number="1" location=".github/workflows/runloop-blueprint-template.json:7">
Add the `-y` flag so this apt install runs non-interactively on the GitHub Actions runner; otherwise the job will hang waiting for confirmation.</violation>
<violation number="2" location=".github/workflows/runloop-blueprint-template.json:8">
Include the `-y` flag so this apt install remains non-interactive; without it the workflow will block waiting for user input.</violation>
</file>
<file name="docs/ide-extensions/agent/quick-start.mdx">
<violation number="1" location="docs/ide-extensions/agent/quick-start.mdx:74">
The sentence updates the first clause to refer to “Agent” but still says “Agent mode” later in the same line, leaving readers with mixed terminology; please align both references for consistency.</violation>
</file>
<file name=".github/workflows/continue-general-review.yaml">
<violation number="1" location=".github/workflows/continue-general-review.yaml:6">
Limiting the push trigger to the personal `nate/fix-wf` branch stops the general review workflow from running on `main`, removing coverage for the default branch.</violation>
</file>
<file name="docs/hub/source-control.mdx">
<violation number="1" location="docs/hub/source-control.mdx:3">
The updated description drops the words identifying the GitHub Action, leaving the sentence as “Continue makes this easy with a that automatically…”. This removes the actionable reference to the automation and produces an incomplete sentence, degrading the documentation clarity.</violation>
</file>
<file name="docs/customization/overview.mdx">
<violation number="1" location="docs/customization/overview.mdx:3">
The new page description claims this guide covers context providers, but the article never mentions them. Please either remove context providers from the description or add matching content so the summary reflects the page.</violation>
</file>
<file name="core/util/paths.ts">
<violation number="1" location="core/util/paths.ts:263">
Writing config.yaml without reapplying 0o600 permissions leaves the file world-readable on Unix systems, exposing user secrets. Please keep toggling permissions after writes to maintain least-privilege access.</violation>
</file>
<file name="core/tools/implementations/requestRule.ts">
<violation number="1" location="core/tools/implementations/requestRule.ts:11">
This tool previously threw a ContinueError so missing rules surfaced with a structured reason; switching to a plain Error makes core/core.ts classify it as Unknown, so callers lose the specific context when a rule can’t be found. Please keep throwing a ContinueError with the appropriate reason so the tool error contract stays intact.</violation>
</file>
<file name="core/tools/implementations/createNewFile.ts">
<violation number="1" location="core/tools/implementations/createNewFile.ts:40">
Use `ContinueError` here so callers receive the specific path-resolution failure reason instead of `Unknown`.</violation>
</file>
<file name="core/nextEdit/providers/MercuryCoderNextEditProvider.ts">
<violation number="1" location="core/nextEdit/providers/MercuryCoderNextEditProvider.ts:46">
Using `message.lastIndexOf("\n\n```")` breaks extraction for standard markdown blocks because the closing fence usually appears as `\n````, so the end index becomes -1 and the result slices off the message tail instead of the fence. Please keep the single newline variant.</violation>
</file>
<file name="docs/customize/model-providers/top-level/openrouter.mdx">
<violation number="1" location="docs/customize/model-providers/top-level/openrouter.mdx:7">
This OpenRouter setup doc now points readers to Inception models, which is unrelated and breaks the guidance. Please restore the link to the OpenRouter models directory.</violation>
</file>
<file name="docs/customization/models.mdx">
<violation number="1" location="docs/customization/models.mdx:103">
The URL for the gpt-oss-20b link is missing the leading "h", producing a broken link in the docs. Please correct it to use a valid https URL.</violation>
</file>
<file name="core/indexing/docs/crawlers/DocsCrawler.test.ts">
<violation number="1" location="core/indexing/docs/crawlers/DocsCrawler.test.ts:22">
Re-enabling this suite makes the network-dependent DocsCrawler tests run on every CI run, reintroducing the flaky integration behavior they were previously skipped for. Please keep the suite skipped (or refactor the tests to be hermetic) before enabling.</violation>
</file>
<file name="extensions/cli/src/hubLoader.ts">
<violation number="1" location="extensions/cli/src/hubLoader.ts:145">
Fetching hub packages without the Authorization header will make every private package download fail—this regresses the existing auth flow that added the bearer token before the request.</violation>
</file>
<file name="docs/customize/deep-dives/prompts.mdx">
<violation number="1" location="docs/customize/deep-dives/prompts.mdx:62">
This sentence now tells readers to add the prompt to an "agent", but the very next snippet (and the config schema) demonstrate that the `prompts:` block belongs at the top level of `config.yaml`. This misdirects users configuring prompts locally and should continue to say "config".</violation>
</file>
<file name="extensions/cli/src/configLoader.ts">
<violation number="1" location="extensions/cli/src/configLoader.ts:92">
Returning a saved file URI without checking that the file still exists causes loadConfiguration to crash on stale file:// URIs instead of falling back to defaults. Please restore the existence guard before selecting the saved-uri source.</violation>
</file>
<file name="core/util/index.ts">
<violation number="1" location="core/util/index.ts:202">
`removeCodeBlocksAndTrim` used to strip `<think>…</think>` segments, but this revision now leaves them in place. Callers like `sanitizeMessageForTTS` will pass those tags straight into speech synthesis, leaking hidden reasoning or placeholder text to users. Please continue removing `<think>` blocks here.</violation>
</file>
<file name=".github/actions/setup-component/action.yml">
<violation number="1" location=".github/actions/setup-component/action.yml:34">
With this change the include-root path now only restores the cache, but never runs npm ci on a cache miss. On a fresh runner node_modules stays empty, so any workflow that sets include-root=true and expects root packages will fail.</violation>
</file>
<file name="core/tools/implementations/lsTool.ts">
<violation number="1" location="core/tools/implementations/lsTool.ts:21">
Switching to resolveRelativePathInDir means absolute and tilde-prefixed directory paths are no longer supported. This helper only resolves paths relative to each workspace root, so an input like `/tmp` (which previously worked) will now be transformed into `tmp` under the workspace and reported as missing, regressing a documented capability.</violation>
</file>
<file name="core/tools/implementations/grepSearch.ts">
<violation number="1" location="core/tools/implementations/grepSearch.ts:66">
Re-throwing the raw error causes callers to miss the structured ContinueError reason; keep wrapping the error so SearchExecutionFailed continues to be surfaced correctly.</violation>
</file>
<file name="actions/general-review/action.yml">
<violation number="1" location="actions/general-review/action.yml:38">
The permission gate now trusts author_association alone, so outside collaborators with only triage/read access (still classified as COLLABORATOR) can trigger the workflow. Please restore a check that enforces write-level permissions before setting SHOULD_RUN=true.</violation>
</file>
<file name="core/context/providers/URLContextProvider.ts">
<violation number="1" location="core/context/providers/URLContextProvider.ts:69">
Returning [] here swallows every fetch/parse error, so callers now get a silent empty result instead of being notified of the failure. Please rethrow after logging so upstream can handle the error.</violation>
</file>
<file name="core/llm/llms/Anthropic.ts">
<violation number="1" location="core/llm/llms/Anthropic.ts:161">
Sending a thinking block with `signature: ""` breaks Anthropic requests because signature-less thinking messages are no longer filtered out when converting chat history back into Anthropic blocks, causing the API to reject the payload.</violation>
</file>
<file name="extensions/cli/src/services/ConfigService.ts">
<violation number="1" location="extensions/cli/src/services/ConfigService.ts:72">
Switching configs no longer passes the agent file state into `configEnhancer.enhanceConfig`, so any rules/prompts/models defined in the agent file are dropped after a switch or reload. Please fetch the agent file state (e.g. from `serviceContainer`) and pass it into `enhanceConfig` here to preserve those injections.</violation>
<violation number="2" location="extensions/cli/src/services/ConfigService.ts:109">
When updating the config path we now store `result.config` directly, so agent-file-derived rules/prompts/models are lost after the path change. Please run the result through `configEnhancer.enhanceConfig` with the current `AgentFileServiceState` before calling `setState` so those injections persist.</violation>
<violation number="3" location="extensions/cli/src/services/ConfigService.ts:130">
After removing the `setState` override, this call no longer updates the `SERVICE_NAMES.CONFIG` entry in `serviceContainer`, leaving consumers that call `serviceContainer.get` with the previous config after a switch/reload. Please invoke `serviceContainer.set(SERVICE_NAMES.CONFIG, this.getState())` when updating the state here (as is still done in `updateConfigPath`).</violation>
</file>
<file name="extensions/cli/src/services/AgentFileService.ts">
<violation number="1" location="extensions/cli/src/services/AgentFileService.ts:57">
Returning here without calling setState bypasses the AgentFileService.setState override, so the update service never receives the initialized agent file state. Please restore the setState call before returning.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| </Steps> | ||
|
|
||
| ## Running Continue CLI with Notion API | ||
| ## Running Continue AI Agent with Notion API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The section below still documents Continue CLI usage, so renaming this heading to “Continue AI Agent” misleads readers about which product or interface the instructions cover. Please keep the heading aligned with the CLI-focused content.
Prompt for AI agents
Address the following comment on docs/guides/notion-continue-guide.mdx at line 115:
<comment>The section below still documents Continue CLI usage, so renaming this heading to “Continue AI Agent” misleads readers about which product or interface the instructions cover. Please keep the heading aligned with the CLI-focused content.</comment>
<file context>
@@ -112,7 +112,7 @@ Before starting, ensure you have:
</Steps>
-## Running Continue CLI with Notion API
+## Running Continue AI Agent with Notion API
<Card title="🚀 Choose Your Interface" icon="zap">
</file context>
| ## Running Continue AI Agent with Notion API | |
| ## Running Continue CLI with Notion API |
| "sudo apt install -y ripgrep" | ||
| "sudo apt install -y ripgrep", | ||
| "sudo apt install asciinema", | ||
| "sudo apt install expect" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include the -y flag so this apt install remains non-interactive; without it the workflow will block waiting for user input.
Prompt for AI agents
Address the following comment on .github/workflows/runloop-blueprint-template.json at line 8:
<comment>Include the `-y` flag so this apt install remains non-interactive; without it the workflow will block waiting for user input.</comment>
<file context>
@@ -3,6 +3,8 @@
- "sudo apt install -y ripgrep"
+ "sudo apt install -y ripgrep",
+ "sudo apt install asciinema",
+ "sudo apt install expect"
]
}
</file context>
| "sudo apt install expect" | |
| "sudo apt install -y expect" |
| "sudo apt update", | ||
| "sudo apt install -y ripgrep" | ||
| "sudo apt install -y ripgrep", | ||
| "sudo apt install asciinema", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the -y flag so this apt install runs non-interactively on the GitHub Actions runner; otherwise the job will hang waiting for confirmation.
Prompt for AI agents
Address the following comment on .github/workflows/runloop-blueprint-template.json at line 7:
<comment>Add the `-y` flag so this apt install runs non-interactively on the GitHub Actions runner; otherwise the job will hang waiting for confirmation.</comment>
<file context>
@@ -3,6 +3,8 @@
"sudo apt update",
- "sudo apt install -y ripgrep"
+ "sudo apt install -y ripgrep",
+ "sudo apt install asciinema",
+ "sudo apt install expect"
]
</file context>
| "sudo apt install asciinema", | |
| "sudo apt install -y asciinema", |
| ## How to Give Agent Permission | ||
|
|
||
| By default, Agent mode will ask permission when it wants to use a tool. Click `Continue` to allow Agent mode to proceed with the tool call or `Cancel` to reject it. | ||
| By default, Agent will ask permission when it wants to use a tool. Click `Continue` to allow Agent mode to proceed with the tool call or `Cancel` to reject it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sentence updates the first clause to refer to “Agent” but still says “Agent mode” later in the same line, leaving readers with mixed terminology; please align both references for consistency.
Prompt for AI agents
Address the following comment on docs/ide-extensions/agent/quick-start.mdx at line 74:
<comment>The sentence updates the first clause to refer to “Agent” but still says “Agent mode” later in the same line, leaving readers with mixed terminology; please align both references for consistency.</comment>
<file context>
@@ -57,21 +57,21 @@ You can switch to `Agent` in the mode selector below the chat input box. The mod
+## How to Give Agent Permission
-By default, Agent mode will ask permission when it wants to use a tool. Click `Continue` to allow Agent mode to proceed with the tool call or `Cancel` to reject it.
+By default, Agent will ask permission when it wants to use a tool. Click `Continue` to allow Agent mode to proceed with the tool call or `Cancel` to reject it.

</file context>
| By default, Agent will ask permission when it wants to use a tool. Click `Continue` to allow Agent mode to proceed with the tool call or `Cancel` to reject it. | |
| By default, Agent will ask permission when it wants to use a tool. Click `Continue` to allow Agent to proceed with the tool call or `Cancel` to reject it. |
| push: | ||
| branches: | ||
| - main | ||
| - nate/fix-wf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limiting the push trigger to the personal nate/fix-wf branch stops the general review workflow from running on main, removing coverage for the default branch.
Prompt for AI agents
Address the following comment on .github/workflows/continue-general-review.yaml at line 6:
<comment>Limiting the push trigger to the personal `nate/fix-wf` branch stops the general review workflow from running on `main`, removing coverage for the default branch.</comment>
<file context>
@@ -3,7 +3,7 @@ name: Continue General Review
push:
branches:
- - main
+ - nate/fix-wf
pull_request:
types: [opened, ready_for_review]
</file context>
| - nate/fix-wf | |
| - main |
| type: "thinking", | ||
| thinking: message.content, | ||
| signature, | ||
| signature: message.signature ?? "", // TODO - unsafe signature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sending a thinking block with signature: "" breaks Anthropic requests because signature-less thinking messages are no longer filtered out when converting chat history back into Anthropic blocks, causing the API to reject the payload.
Prompt for AI agents
Address the following comment on core/llm/llms/Anthropic.ts at line 161:
<comment>Sending a thinking block with `signature: ""` breaks Anthropic requests because signature-less thinking messages are no longer filtered out when converting chat history back into Anthropic blocks, causing the API to reject the payload.</comment>
<file context>
@@ -158,30 +153,20 @@ class Anthropic extends BaseLLM {
type: "thinking",
thinking: message.content,
- signature,
+ signature: message.signature ?? "", // TODO - unsafe signature
},
];
</file context>
| logger.debug("Applied injected configuration"); | ||
| } | ||
|
|
||
| this.setState({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After removing the setState override, this call no longer updates the SERVICE_NAMES.CONFIG entry in serviceContainer, leaving consumers that call serviceContainer.get with the previous config after a switch/reload. Please invoke serviceContainer.set(SERVICE_NAMES.CONFIG, this.getState()) when updating the state here (as is still done in updateConfigPath).
Prompt for AI agents
Address the following comment on extensions/cli/src/services/ConfigService.ts at line 130:
<comment>After removing the `setState` override, this call no longer updates the `SERVICE_NAMES.CONFIG` entry in `serviceContainer`, leaving consumers that call `serviceContainer.get` with the previous config after a switch/reload. Please invoke `serviceContainer.set(SERVICE_NAMES.CONFIG, this.getState())` when updating the state here (as is still done in `updateConfigPath`).</comment>
<file context>
@@ -70,258 +48,97 @@ export class ConfigService
+ logger.debug("Applied injected configuration");
+ }
+
+ this.setState({
+ config,
+ configPath: newConfigPath,
</file context>
| const state = await this.loadConfig(init); | ||
| // Use the new streamlined config loader | ||
| const { loadConfiguration } = await import("../configLoader.js"); | ||
| const result = await loadConfiguration( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When updating the config path we now store result.config directly, so agent-file-derived rules/prompts/models are lost after the path change. Please run the result through configEnhancer.enhanceConfig with the current AgentFileServiceState before calling setState so those injections persist.
Prompt for AI agents
Address the following comment on extensions/cli/src/services/ConfigService.ts at line 109:
<comment>When updating the config path we now store `result.config` directly, so agent-file-derived rules/prompts/models are lost after the path change. Please run the result through `configEnhancer.enhanceConfig` with the current `AgentFileServiceState` before calling `setState` so those injections persist.</comment>
<file context>
@@ -70,258 +48,97 @@ export class ConfigService
- const state = await this.loadConfig(init);
+ // Use the new streamlined config loader
+ const { loadConfiguration } = await import("../configLoader.js");
+ const result = await loadConfiguration(
+ authConfig,
+ newConfigPath,
</file context>
| agentFileState?.agentFile || | ||
| (injectedConfigOptions && this.hasInjectedConfig(injectedConfigOptions)) | ||
| ) { | ||
| config = await configEnhancer.enhanceConfig( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching configs no longer passes the agent file state into configEnhancer.enhanceConfig, so any rules/prompts/models defined in the agent file are dropped after a switch or reload. Please fetch the agent file state (e.g. from serviceContainer) and pass it into enhanceConfig here to preserve those injections.
Prompt for AI agents
Address the following comment on extensions/cli/src/services/ConfigService.ts at line 72:
<comment>Switching configs no longer passes the agent file state into `configEnhancer.enhanceConfig`, so any rules/prompts/models defined in the agent file are dropped after a switch or reload. Please fetch the agent file state (e.g. from `serviceContainer`) and pass it into `enhanceConfig` here to preserve those injections.</comment>
<file context>
@@ -70,258 +48,97 @@ export class ConfigService
+ agentFileState?.agentFile ||
+ (injectedConfigOptions && this.hasInjectedConfig(injectedConfigOptions))
+ ) {
+ config = await configEnhancer.enhanceConfig(
+ config,
+ injectedConfigOptions,
</file context>
|
|
||
| // Set the basic agent file state | ||
| this.setState({ | ||
| return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning here without calling setState bypasses the AgentFileService.setState override, so the update service never receives the initialized agent file state. Please restore the setState call before returning.
Prompt for AI agents
Address the following comment on extensions/cli/src/services/AgentFileService.ts at line 57:
<comment>Returning here without calling setState bypasses the AgentFileService.setState override, so the update service never receives the initialized agent file state. Please restore the setState call before returning.</comment>
<file context>
@@ -71,40 +54,20 @@ export class AgentFileService
- // Set the basic agent file state
- this.setState({
+ return {
+ agentFileService: this,
agentFile,
</file context>
|
@Patrick-Erichsen Pulled from main and merged, previously rebased too but not sure why the checks fails and these are very much random. Closing this and creating a new PR: #8383. Same scenario here, too. |
I have read the CLA Document and I hereby sign the CLA
Description
Handle malformed data URLs in Bedrock, Anthropic, Gemini, and Ollama adapters. Currently, they use unsafe array destructuring when parsing data URLs causing runtime crashes on malformed input or unexpected results even if the parsed data is not correct base64.
Data URLs can be malformed due to various reasons, like, copy/paste from applications generating invalid URLs, user provided code containing malformed base64 URLs or maybe a network connection during image uploads.
This fix:
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
[ When applicable, please include a short screen recording or screenshot - this makes it much easier for us as contributors to review and understand your changes. See this PR as a good example. ]
Tests
[ What tests were added or updated to ensure the changes work as expected? ]
Summary by cubic
Improves image data URL parsing across Anthropic, Bedrock, Gemini, and Ollama to prevent crashes on malformed input and return clearer errors.