Reject non-positive/fractional limit values in getLLMDecisionWithVision's
Anthropic branch, matching the positive-safe-integer rule already enforced
in parseSearchIntent. Validate and sanitize generateFieldLabelsBatch's
Anthropic-branch label mapping before returning it: reject non-string,
blank, and __proto__/constructor/prototype values, since callers use each
value as an object key downstream (renamedFields[semanticLabel] = ...).
Requested by maintainer review on PR #1189. The forced tool_choice call in
isMultiSitePrompt's Anthropic branch left too little headroom at 20 tokens,
risking truncation before the tool_use block completes.
Reject __proto__/constructor/prototype field names and use a null-prototype
target in filterFieldsByIntent's Anthropic branch to prevent prototype
pollution from untrusted tool output. Add explicit type validation for
first/second/reason/limit in getLLMDecisionWithVision's Anthropic branch,
since input_schema isn't runtime-enforced without strict mode. Reject
non-positive and non-safe-integer limit values in both parseSearchIntent
validation blocks (Anthropic branch and shared ollama/openai tail).
All 3 verified against current code and the actual failure modes before
fixing, not applied blindly from the bot's suggestion text.
1. getLLMDecisionWithVision: the Anthropic image block declared
media_type: 'image/png' while the screenshot is captured as JPEG
(page.screenshot({ type: 'jpeg' })). This is the exact mismatch already
disclosed (by me) in the PR's "Note to Reviewers" before CodeRabbit
caught it independently, confirmed against Anthropic's own docs that a
media_type mismatch can 400 before the forced tool call ever runs.
Fixed to 'image/jpeg'. The identical bug also exists in the untouched
openai branch's `data:image/png;base64,...` URL - left as-is and
flagged, not silently fixed, since that branch is explicitly out of
this PR's scope.
2. parseSearchIntent: the validation `!intent.searchQuery ||
!intent.extractionGoal` is a truthiness check, not a type check. Since
callAnthropicWithTool returns raw tool_use.input with no strict-mode
validation, a non-string value (e.g. an object) passes this guard, and
downstream `encodeURIComponent(query)` (workflowEnricher.ts, both
performDuckDuckGoSearch and performDuckDuckGoMultiSearch) silently
turns it into the literal string "[object Object]" as a search query.
Verified this exact call chain before fixing. Replaced with a guard
that requires searchQuery/extractionGoal to be non-blank strings and
limit (when present) to be an integer. Applied to both the anthropic
branch's copy of this check AND the shared ollama/openai JSON-parsing
branch's identical copy, per CodeRabbit's own comment text ("Apply the
same validation to the JSON parsing branch") - not my own scope
expansion, the finding explicitly asked for both.
3. selectMultipleUrlsFromResults: the per-index collection loop had no
cap on `result.length`, so a tool response returning more valid,
unique-domain indices than the caller's `maxSites`/`cap` would collect
all of them instead of stopping at cap - e.g. 5 valid indices with
cap=4 would produce 5 downstream workflows instead of 4. Added
`if (result.length >= cap) break;` at the top of the loop, matching
CodeRabbit's exact suggested diff. This identical bug also exists,
unchanged, in the original shared-tail copy of this same loop (which
predates this PR and serves ollama/openai) - CodeRabbit's finding did
not ask for that copy to be fixed too, so it's flagged, not touched.
Verified: full project tsc --noEmit clean. A targeted test reproduced each
finding's exact described failure mode against the pre-fix code path
conceptually and confirmed the fix resolves it: searchQuery as an object,
as an array, as an empty/whitespace string, limit as a fraction, and limit
as a numeric string are all now correctly rejected (falling back to the
existing regex heuristic, not silently proceeding); valid inputs including
an omitted limit still succeed unchanged; 5 valid unique-domain indices
against cap=4 now correctly returns exactly 4, and a below-cap selection
is unaffected. Re-ran the full 9-site fallback regression afterward to
confirm the shared-code changes (finding #2) didn't affect any of the
other 8 sites.
All 9 anthropic branches now go through callAnthropicWithTool; the raw
`new Anthropic(...)` client construction that used to happen per-call-site
is gone as of the site #9 migration (372ded1d). The top-level
`import Anthropic from '@anthropic-ai/sdk'` had zero remaining references
- confirmed via `grep -n '\bAnthropic\b'` before removing it.
Migrates the anthropic branch of selectMultipleUrlsFromResults (site #9 of
9, issue #1178) off regex fence-stripping + JSON.parse onto
callAnthropicWithTool with a forced select_multiple_urls tool. This is the
last of the 9 call sites - the migration is now complete.
- Appends selectMultipleUrlsTool to server/src/sdk/anthropicTools.ts.
required: ['selectedIndices'] matches the sibling Ollama jsonSchema
exactly (reasoning stays optional, same precedent).
- Typed as callAnthropicWithTool<any>, same rationale as sites #3/#7/#8:
`!Array.isArray(decision.selectedIndices)` is a structural/existence
check that a precise array type would risk making look dead.
- This site has the most fallback-triggering paths of the 9: a local
`fallback()` closure (declared outside the try block, using `searchResults`
and `cap` from the enclosing scope) is called INLINE in three places, not
via throw - (1) the pre-try `cap <= 1` short-circuit, untouched, (2) when
selectedIndices is missing/empty, (3) when the per-index filtering loop
(type/range check + domain dedup via `new Set<string>()`) leaves fewer
than 2 results. Only the fourth path - an actual thrown error - routes
through the existing outer catch. Ported the filtering loop and both
inline fallback() calls verbatim, reusing the original `seen`/`result`
variable names in the nested block scope (matching every prior site's
convention - an earlier draft of this commit accidentally renamed them
to seenAnthropic/resultAnthropic to "avoid a collision" that block
scoping already prevents; reverted before this landed, for consistency
with sites #2/#3/#5/#6/#7's identical reuse of decision/labelMapping/etc.
in their own nested blocks).
- Preserves max_tokens 256 and temperature 0.1 exactly as before.
Verified: full project tsc --noEmit clean (including confirming the
reused seen/result names don't collide with the shared tail's own
same-named declarations, exactly as precedent predicted). Mocked runtime
check (ts-node) covered 8 scenarios: the cap<=1 short-circuit correctly
never invokes the helper at all (asserted zero calls); a valid 3-index
selection returns entries in the right order with the mocked reasoning;
a selection including a duplicate-domain index correctly dedupes (keeps
the first occurrence, drops the later one); a non-array selectedIndices
correctly triggers the inline fallback() (not a throw); an empty array
does the same; an array of entirely out-of-range/wrong-type indices
filters down to zero results and correctly falls back; a single
valid-but-lone result also falls back (the >= 2 threshold); and an
outright helper throw is caught by the pre-existing catch and falls back
identically.
This completes the tool_choice migration for all 9 Anthropic call sites in
workflowEnricher.ts. `new Anthropic(` now has zero remaining call sites in
this file - the top-level `import Anthropic from '@anthropic-ai/sdk'` is
consequently dead code. Deliberately NOT removed in this commit to keep it
scoped to site #9 only, matching every prior commit's scope discipline;
flagging as an explicit, obvious follow-up rather than silently leaving it
dangling or silently sneaking an unrelated cleanup into this diff.
Migrates the anthropic branch of isMultiSitePrompt (site #8 of 9, issue
with a forced classify_multi_site_prompt tool.
- Appends classifyMultiSitePromptTool to server/src/sdk/anthropicTools.ts.
required: ['multiSite'] matches the sibling Ollama jsonSchema exactly.
- This site has a distinctive 3-way control flow with NO hard throw:
1. Valid boolean multiSite -> logs `isMultiSitePrompt: <value>`, returns
it directly.
2. Response received but multiSite missing/wrong type -> SILENTLY
returns false (no log at all - the original only logs on this path
via nothing, since there's no explicit branch for it, it just falls
through). Reproduced this exact silence rather than adding a log line
that wasn't there before.
3. Helper throws -> propagates to the function's EXISTING outer catch
(unchanged), which logs the exact original warning message and falls
through to the EXISTING final `return false` (unchanged).
- Did NOT simplify this to `return decision.multiSite` - a malformed,
non-boolean multiSite (e.g. the string "true") would then be returned as
a truthy-but-wrong value instead of correctly degrading to false. Kept
the `typeof decision.multiSite === 'boolean'` guard verbatim.
- The anthropic branch now always returns from within its own block
(either the boolean or the inline `return false`), so it never falls
through to the shared regex/JSON.parse tail - verified this doesn't
break TypeScript's definite-assignment analysis for the still-shared
`llmResponse` variable used by ollama/openai.
- max_tokens 20 and temperature 0 preserved unchanged (this call already
asked for JSON-shaped text, like site #8's sibling isMultiSitePrompt
itself - no budget bump needed here, unlike site #4's free-text case).
Verified: full project tsc --noEmit clean. Mocked runtime check (ts-node)
covered 5 scenarios, with logger.info/logger.warn also monkey-patched to
assert directly on which log lines fire (not just the return value):
multiSite: true logs and returns true; multiSite: false ALSO logs (info
line fires for both booleans) and returns false; an empty decision object
returns false with ZERO log calls, matching the original's silence for
this specific case; multiSite as a non-boolean string ("true") is
correctly rejected by the typeof guard and also returns false silently -
confirming a truthy-coercion bug isn't hiding here; and a thrown helper
error is caught by the pre-existing catch, logs the exact original warning
text, and returns false via the pre-existing fallthrough.
Migrates the anthropic branch of selectBestUrlFromResults (site #7 of 9,
issue #1178) off regex fence-stripping + JSON.parse onto callAnthropicWithTool
with a forced select_best_url tool.
- Appends selectBestUrlTool to server/src/sdk/anthropicTools.ts.
required: ['selectedIndex', 'confidence', 'reasoning'] matches the
sibling Ollama jsonSchema exactly - no precedent divergence here.
- Typed as callAnthropicWithTool<any>, not a precise type - unlike site
#6's pure truthiness checks, this site's hard-throw explicitly compares
`decision.selectedIndex === undefined`. An explicit undefined comparison
is an existence/structural check (same category as site #3's
Array.isArray), not a plain truthy check, so a `number` type annotation
here would risk making the check read as dead code to a future
maintainer even though it is not actually enforced at runtime.
- Anthropic branch returns directly; this function's shared tail does
JSON.parse, so early-return is correct (matching sites #1/#3/#5/#6).
- The length===1 short-circuit before the try block, and the catch
block's searchResults[0]-based fallback (confidence 0.6, fixed
reasoning string), are both untouched.
- Preserves max_tokens 256 and temperature 0.1 exactly as before.
Verified: full project tsc --noEmit clean. Mocked runtime check (ts-node)
covered 8 scenarios, most importantly: confirmed the length===1
short-circuit never invokes the helper at all (asserted zero calls), and
confirmed selectedIndex: 0 - a value a careless truthy-check refactor could
easily misread as "missing" - is correctly honored as a valid index rather
than triggering the invalid-index fallback. Also covered undefined,
negative, and out-of-bounds selectedIndex all correctly throwing
internally and falling back to the first search result with the exact
fixed confidence/reasoning, a valid index with confidence/reasoning
omitted correctly using the `||` defaults (0.5 / 'No reasoning provided'),
and an outright helper throw falling back identically.
Migrates the anthropic branch of parseSearchIntent (site #6 of 9, issue
with a forced parse_search_intent tool.
- Appends parseSearchIntentTool to server/src/sdk/anthropicTools.ts.
required: ['searchQuery', 'extractionGoal'] only, matching the sibling
Ollama jsonSchema exactly - limit stays optional since the call site
treats an absent key and an explicit null identically.
- Typed with a precise generic ({searchQuery: string; extractionGoal:
string; limit: number | null}), not <any> - unlike sites #2/#3/#5, this
site's hard-throw check (`!intent.searchQuery || !intent.extractionGoal`)
is a value/truthiness check, not a structural one. A `string` type
annotation doesn't make a truthy-check on a possibly-empty string look
redundant the way it would for e.g. an Array.isArray check, so the
precise type doesn't undermine anything here (same reasoning as site #1).
- Anthropic branch returns directly; this function's shared tail does
JSON.parse, so early-return is correct here (matching sites #1/#3/#5).
- The catch block's REGEX-BASED heuristic fallback (extracting a search
query via /from\s+([^,\.]+)/i and a limit via /(\d+)/ straight from the
raw prompt) is untouched - this is the only one of the 9 sites whose
fallback isn't a static constant or identity mapping.
- Preserves max_tokens 256 and temperature 0.1 exactly as before.
Verified: full project tsc --noEmit clean. Mocked runtime check (ts-node)
covered 6 scenarios: valid full response, a null limit passed through
correctly, an empty searchQuery correctly throwing internally and falling
back to the regex heuristic (confirmed it extracts the right search term
AND the right numeric limit from the raw prompt, and that the fallback's
extractionGoal is the full raw prompt - not the mock's rejected value), an
empty extractionGoal taking the same throw-and-fallback path, a prompt with
no "from X" clause correctly degrading the heuristic's searchQuery to the
first 50 characters, and an outright helper throw falling back identically.
Migrates the anthropic branch of verifyWorkflowOutput (site #5 of 9, issue
with a forced verify_extraction_match tool.
- Appends verifyExtractionMatchTool to server/src/sdk/anthropicTools.ts.
required: ['matches'] only, matching the sibling Ollama jsonSchema
exactly (this was corrected during blueprint review from an earlier
draft that over-required confidence/reasoning too - reverted to match
the only real precedent and the call site's own soft-default handling).
- Typed as callAnthropicWithTool<any>, same rationale as sites #2/#3: the
call site treats matches/confidence/reasoning as optional-with-defaults
via typeof guards (`typeof decision.matches === 'boolean' ? ... : true`,
etc.), not hard requirements. A narrower type would make those guards
look redundant when they are not actually enforced at runtime.
- Anthropic branch returns directly (this function's shared tail does
JSON.parse, same as sites #1/#3, so early-return is correct here -
unlike site #4's fall-through, which was correct there because that
tail is pure string processing with no JSON.parse).
- Preserves the exact defensive extraction and its logger.info result line
verbatim, just reading from `decision` instead of `parsed`.
- Preserves max_tokens 256 and temperature 0.1 exactly as before.
Verified: full project tsc --noEmit clean. Mocked runtime check (ts-node)
covered 6 scenarios, most importantly distinguishing the two different
failure modes this function has: a decision that resolves successfully but
is empty/malformed (`{}`) takes the SUCCESS path and produces
reasoning: '' via the field-level typeof defaults, while a thrown error
from the helper takes the CATCH path and produces the pre-declared
fallback constant's distinct reasoning: 'Verification skipped (LLM
unavailable)' - confirmed these remain genuinely different code paths
producing different values, not collapsed into one. Also verified matches
defaulting to true on wrong-type input, confidence defaulting to 0.5 on
wrong-type input, and that each default is independent (a bad `matches`
doesn't affect a valid `confidence` in the same response, and vice versa).
Migrates the anthropic branch of generateListName (site #4 of 9, issue
set_list_name tool.
- Appends setListNameTool to server/src/sdk/anthropicTools.ts.
- This is the only one of the 9 sites converting a free-text completion
(not a JSON-shaped one) into a structured tool call - the prompt
previously asked for bare text like "Product Listings", not JSON.
- Unlike sites #1-3, this branch does NOT early-return: the shared tail
here is pure string post-processing (quote-strip, first-line-only,
length validation, Title-Case) with no JSON.parse, so it works
identically whether it receives raw text (ollama/openai) or
decision.listName (anthropic). Assigning `llmResponse = decision.listName`
and falling through to the unchanged shared tail is the smaller, more
faithful port than duplicating that logic inline would have been.
- max_tokens bumped 20 -> 80 for the anthropic branch only (ollama/openai
untouched). The prior budget was sized for bare-text output with zero
JSON overhead; a {"listName": "..."} tool call needs headroom on top of
a name that can legally run up to 50 chars per the existing validation.
Left uncorrected, this would risk stop_reason: max_tokens truncation on
longer names, converting what worked before into a guaranteed "List 1"
fallback for exactly the names most likely to need one.
- temperature 0.1 preserved as before.
Verified: full project tsc --noEmit clean. Mocked runtime check (ts-node)
covered 7 scenarios: clean valid name, quote-wrapped name correctly
stripped, multi-line response correctly truncated to its first line, an
empty name correctly throws internally and falls back to "List 1" via the
EXISTING (llmResponse || '').trim() guard - unchanged, no new guard code
needed - an undefined listName field degrades through that same existing
guard identically, an oversized (>50 char) name correctly throws and falls
back, and a thrown helper error (e.g. simulated truncation) falls back
identically. Also asserted opts.maxTokens === 80 specifically, to catch
any regression of the budget bump.
Migrates the anthropic branch of filterFieldsByIntent (site #3 of 9,
issue #1178) off regex fence-stripping + JSON.parse onto callAnthropicWithTool
with a forced filter_fields_by_intent tool.
- Appends filterFieldsByIntentTool to server/src/sdk/anthropicTools.ts.
required: ['selectedFields','confidence','reasoning'] matches the sibling
Ollama jsonSchema exactly - no precedent divergence here.
- Typed as callAnthropicWithTool<any>, same rationale as site #2: this
function has HARD throws on invalid shape (`!Array.isArray(selectedFields)`,
confidence type/range check) that are load-bearing for the fallback
mechanism. A narrower generic type would let TypeScript claim those
checks are unreachable when they are not actually guaranteed at runtime
(non-strict tool, no server-side validation). Ported both throws and the
unknown-field-name warn-and-skip loop verbatim, reading from `decision`
instead of `filterResult`.
- Anthropic branch returns directly; shared regex/JSON.parse tail untouched,
still serves ollama/openai.
- Preserves max_tokens 1024 and temperature 0.1 exactly as before.
Verified: full project tsc --noEmit clean. Mocked runtime check (ts-node)
covered 7 scenarios: valid high-confidence success, valid low-confidence
(needsUserConfirmation flips true), non-array selectedFields correctly
throws and falls back to the full labeledFields object, out-of-range
confidence correctly throws and falls back, wrong-type confidence correctly
throws and falls back, an unknown field name in selectedFields is warned
and silently dropped rather than crashing, and an outright helper throw
falls back identically. Also asserted the exact opts reaching the helper.
Migrates the anthropic branch of generateFieldLabelsBatch (site #2 of 9,
issue #1178) off regex fence-stripping + JSON.parse onto callAnthropicWithTool
with a forced assign_field_labels tool.
- Appends assignFieldLabelsTool to server/src/sdk/anthropicTools.ts.
- Typed as callAnthropicWithTool<any>, deliberately - the original code's
defensive `if (parsedResponse.fieldLabels) {...} else { labelMapping =
parsedResponse }` fallback exists precisely because the response shape
isn't guaranteed even under the tool's required schema (non-strict tool,
no server-side validation). Typing this call more narrowly would let
TypeScript claim decision.fieldLabels is always defined when it isn't,
silently defeating the exact check it's supposed to preserve. Ported that
defensive block and the missingLabels identity-fill logic verbatim, just
reading from `decision` instead of `parsedResponse`.
- Anthropic branch now returns directly, same early-return pattern as site
#1; the shared regex/JSON.parse tail is untouched and still serves
ollama/openai unchanged.
- Preserves max_tokens 2048 and temperature 0.1 exactly as before.
Verified: full project tsc --noEmit clean. Mocked runtime check (ts-node,
monkey-patched callAnthropicWithTool) covered three scenarios: wrapped
{fieldLabels:{...}} success, a flat (unwrapped) success shape to prove the
defensive else-branch genuinely still fires and the missingLabels fill-in
still runs on top of it, and a thrown error correctly falling back to full
identity-mapping via generateFieldLabelsBatch's own catch. Also asserted
the exact opts reaching the helper (tool identity, model, maxTokens,
temperature, apiKey, plain-string userMessage, non-empty system prompt).
Migrates the anthropic branch of getLLMDecisionWithVision (site #1 of 9,
issue #1178) off regex fence-stripping + JSON.parse onto callAnthropicWithTool
with a forced select_group_candidates tool.
- Adds server/src/sdk/anthropicTools.ts with selectGroupCandidatesTool -
first of the 9 tool definitions, will grow with each subsequent site.
- The anthropic branch now returns directly from inside the if/else chain
once its decision is built, instead of falling through to the shared
regex/JSON.parse tail that ollama and openai still use unchanged.
- Preserves the vision content shape exactly (image block + text block,
still carrying the pre-existing jpeg-captured/media_type:'image/png'
mismatch at screenshot capture - not touched, out of scope for this PR).
- Preserves the missing `temperature` param (this was the one call site of
the 9 that never sent it) and the existing claude-3-5-sonnet-20241022
default (stale/retired model ID - also out of scope, filed separately).
- fallbackHeuristicDecision is unchanged and still triggered by the same
outer catch on any thrown error from the new call.
Verified: full project tsc --noEmit clean; a mocked runtime check (ts-node,
monkey-patched callAnthropicWithTool) confirmed the helper receives the
exact expected tool/model/maxTokens/apiKey/userMessage array and no
temperature field, that a successful decision flows correctly through
parseGroupCandidates/buildDecisionFromCandidates, and that a thrown error
still correctly falls back to the existing fallbackHeuristicDecision.
Karishma Shukla