-
Notifications
You must be signed in to change notification settings - Fork 153
fix: scrub blocked URLs from system[] instead of relocating all content #198
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -169,35 +169,33 @@ export function transformBody( | |||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| parsed.system = splitSystem | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // --- Relocate non-core system entries to user messages --- | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Anthropic's API now validates the system prompt for OAuth-authenticated | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // requests that use Claude Code billing. Third-party system prompts | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // (like OpenCode's) trigger a 400 "out of extra usage" rejection when | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // they appear inside the system[] array alongside the identity prefix. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Work-around: keep only the billing header and identity prefix in | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // system[], and prepend all other system content to the first user | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // message where it is functionally equivalent but avoids the check. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const BILLING_PREFIX = "x-anthropic-billing-header" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const keptSystem: SystemEntry[] = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const movedTexts: string[] = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const entry of parsed.system) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const txt = typeof entry === "string" ? entry : (entry.text ?? "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (txt.startsWith(BILLING_PREFIX) || txt.startsWith(SYSTEM_IDENTITY)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| keptSystem.push(entry) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (txt.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| movedTexts.push(txt) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // --- Relocate blocked system entries to user messages --- | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Anthropic's billing validator rejects specific URLs in system[] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // (e.g. the OpenCode GitHub repo URL). Instead of moving ALL | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // non-core system content (which regresses instruction priority | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // and prompt-cache efficiency), only relocate entries that contain | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // a blocked string. Everything else stays in system[]. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const BLOCKED_SYSTEM_STRINGS = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/anomalyco/opencode", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| function isBlocked(text: string): boolean { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return BLOCKED_SYSTEM_STRINGS.some((s) => text.includes(s)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (movedTexts.length > 0 && Array.isArray(parsed.messages)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const firstUser = parsed.messages.find((m) => m.role === "user") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (firstUser) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| parsed.system = keptSystem | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const prefix = movedTexts.join("\n\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof firstUser.content === "string") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| firstUser.content = prefix + "\n\n" + firstUser.content | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (Array.isArray(firstUser.content)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| firstUser.content.unshift({ type: "text", text: prefix }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Scrub blocked strings from system entries in-place rather than | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // relocating entire entries. OpenCode concatenates the full system | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // prompt (identity + agent prompt + env + AGENTS + skills) into a | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // single text block, so moving the whole entry on a substring match | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // would frontload the entire prompt into the user message. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const entry of parsed.system) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| entry.type === "text" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof entry.text === "string" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| isBlocked(entry.text) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const blocked of BLOCKED_SYSTEM_STRINGS) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| entry.text = entry.text.split(blocked).join("") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+191
to
200
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If a system entry contains only the blocked URL (e.g.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/transforms.ts
Line: 191-200
Comment:
**Empty system entry left after full-URL scrub**
If a system entry contains *only* the blocked URL (e.g. `{ type: "text", text: "github.com/anomalyco/opencode" }`), scrubbing reduces `entry.text` to `""`. That empty-text entry is then forwarded to Anthropic's API, which may reject it or silently alter billing/validation logic. The previous implementation explicitly checked `txt.length > 0` before including text in `movedTexts`, preventing this. The new code has no equivalent guard — after the mutation loop, entries with `entry.text === ""` remain in `parsed.system` unchanged.
```suggestion
for (const entry of parsed.system) {
if (
entry.type === "text" &&
typeof entry.text === "string" &&
isBlocked(entry.text)
) {
for (const blocked of BLOCKED_SYSTEM_STRINGS) {
entry.text = entry.text.split(blocked).join("")
}
}
}
// Remove any entries that were reduced to empty strings by scrubbing.
parsed.system = parsed.system.filter(
(e) => !(e.type === "text" && e.text === ""),
)
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
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 test is named
"transformBody relocates multiple non-core system entries to user message as content blocks", but the body now asserts the exact opposite: that all entries stay insystem[](parsed.system.length === 4). This will confuse anyone reading the test output or running it in watch mode — a failing test's name would describe behavior that no longer applies.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!