Harden model-facing decoding against common LLM tool-call quirks (0.2.2) - #6
Merged
Conversation
Three defensive fixes so ordinary model output stops nuking whole calls:
1. Registry coerces declared .number/.bool arguments from strings before
validation: a numeric string ("20") becomes a number and "true"/"false"
(case-insensitive only — not 1/0 or "yes") becomes a bool, at every declared
argument including nested dotted paths. Both the type check and the bridge
then see canonical values. Safety gates like "confirmed" still require an
explicit true/false.
2. JavaScriptExecutionRequest gets a tolerant init(from:): timeoutMs accepts a
number or a numeric string, and — matching the advertised schema where only
code + allowedCapabilities are required — allowedCapabilityKeys, timeoutMs,
and context may be omitted. The synthesized decoder wrongly required all
three, so a valid call that left them out (as the field report did) was
rejected before any JS ran.
3. An unknown/misspelled capability ID in allowedCapabilities now yields a clear
error naming the offending value(s) and pointing at searchJavaScriptAPI,
instead of an opaque Codable failure.
Audited the other model-facing Codable input (JavaScriptAPISearchRequest): just
a code string, nothing to harden.
Adds DefensiveDecodingTests (11 cases) and retargets
registryValidationRejectsWrongArgumentType at a bool (a genuine type mismatch,
not a coerced quirk). 241 tests pass; deterministic evals 49/49.
zac
added a commit
that referenced
this pull request
Jul 26, 2026
Nothing capped output. The whole result was stringified, copied into a Swift String, and re-decoded; every console.log was retained and then copied again into each CodeModeToolError; the events AsyncStream used the default unbounded buffering, so a chatty script whose events the host was not draining buffered without limit. One script could jetsam an iOS host. `CodeModeConfiguration.executionLimits` bounds all of it, and every limit degrades rather than fails — for an LLM consumer a truncated result with a diagnostic beats an out-of-memory kill: - Result size is measured *inside* JS, so an oversized value is never brought across. Over the limit, the output is replaced by a still-parseable descriptor (`__codeModeTruncated`, character count, limit, preview) plus a RESULT_TRUNCATED diagnostic suggesting fs.write + return the path. - Retained logs are capped at the first N with a one-time LOG_LIMIT_REACHED notice; streaming stays live past the cap since it is the retained copy that costs. Over-long messages are elided in the middle, keeping head and tail. - Diagnostics and permission events are capped. - The events stream uses `.bufferingNewest(n)`. Two watchdog findings in the same code path: - The serialization re-arm used `max(timeoutMs, 1000)`, so a 60s execution bought another 60s for JSON.stringify. It is `min` against a fixed 1s budget now, which is what "bounded budget" meant. - `watchdog.termination` was never consulted after the re-arm, so a runaway getter surfaced as `INVALID_RESULT: must be JSON-serializable` and steered the model to the wrong fix. It now reports EXECUTION_TIMEOUT/CANCELLED naming the serialization phase. Review finding P0 #6 and the first two P2 items.
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
1 participant
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.
Defensive fixes prompted by a field failure: an
executeJavaScriptcall was rejected before any JS ran because the model senttimeoutMsas the string"10000"and omittedallowedCapabilityKeys/context. Root cause was strict, schema-misaligned decoding at the model-facing boundary. This hardens that boundary against the common LLM output quirks.Changes
Registry coerces declared
.number/.boolarguments from strings before validation. A numeric string ("20") becomes a number;"true"/"false"(case-insensitive only — not1/0or"yes") becomes a bool. Applied to every declared argument, including nested dotted paths (e.g.options.timeoutMs), so both the type check and the bridge see canonical values. Safety gates likeconfirmedstill require an explicit true/false.JavaScriptExecutionRequestgets a tolerantinit(from:).timeoutMsaccepts a number or a numeric string; and — matching the advertised schema where onlycode+allowedCapabilitiesare required —allowedCapabilityKeys,timeoutMs, andcontextmay now be omitted. The synthesized decoder wrongly required all three, so a valid call that left them out was rejected outright.Unknown/misspelled capability IDs give a clear error. An unrecognized value in
allowedCapabilitiesnow names the offending value(s) and points atsearchJavaScriptAPI, instead of an opaqueCodablefailure.Audited the other model-facing
Codableinput (JavaScriptAPISearchRequest): just acodestring, nothing to harden.Tests
DefensiveDecodingTests(11 cases): numeric-string / number / null / omittedtimeoutMs, minimal{code, allowedCapabilities}, unknown-capability error, Codable round-trip, and registry coercion (numeric string, case-insensitive bools, nested dotted path, and non-coercion of"yes"/1/0/non-numeric).registryValidationRejectsWrongArgumentTypeat a bool (a genuine type mismatch, not a coerced quirk).Intended as release 0.2.2.