* refactor(video_generate): capability-gated dynamic schema — 6 optional args render only when the active provider/model honors them; fleet capability declarations + declaration<->implementation contract tests
* fix(video_gen): H3/Grok/Happy-Horse/Gemini audio is ALWAYS-ON native, not absent — new audio_native family key + audio_always_on capability surfaces as description line (maintainer catch)
* test(video_gen): duration-span test pins the active-model contract — resolved family's real window, short families not inflated, union fallback still spans 30s
Enough1122 review of #94426:
- Legacy Bot tiles persisted before ownerRoute.connectionId existed carry no
connection id, so the local-delete branch (`ownerConnection === 'local'`)
never matched them and they resurrected the deleted profile on relaunch.
Treat the empty connection id as the local connection.
- A route without profile now throws instead of silently falling into the
local-delete branch and dropping nothing remotely owned.
- Pin the 'local' connection spelling with tests covering the legacy
(no id), canonical ('local'), and divergent ('Local') spellings.
- Comment the live-atom filter arms by bucket and unify the duplicated
#94235 call-site comments in sdk/index.ts and delete-profile-dialog.tsx.
Address review findings on #94426:
- Non-route (local profile) deletes now require the tile's owner
connectionId to be 'local' — a same-named bot on another connection
keeps its tile and live conversation.
- Route identity (profile/targetProfile) goes through normalizeProfileKey
like the non-route name, so whitespace-padded identities match on both
paths; profile case stays exact, consistently on both paths.
- Test lint: drop the unused dropTilesForProfile value import (tests call
it via the fresh module namespace) and replace the forbidden typeof
import() type annotation with a type-only namespace import. New tests:
same-name-other-connection survival, whitespace normalization, and
case-exact identity. eslint: 0 errors; typecheck clean.
A Bot Mode bot cloned via 'Clone from profile' resurrects after
deletion: the desktop keeps the bot's chat tile in local storage
(bot-meta cleanup removes the roster entry, but the persisted tile
survives). On relaunch the tile restores, re-dials the deleted
profile's backend, and ensure_hermes_home() re-creates the profile
directory the delete just removed — the empty skeleton with no
config.yaml reported in #94235.
dropTilesForProfile() removes a deleted profile's session-tile
bucket and every Bot Mode tile whose ownerRoute points at it
(exact connection + backend target when a source-scoped route is
given) from memory and local storage, with discard (no Cmd+Shift+T
undo) semantics. Wired into both delete doors: the core
DeleteProfileDialog and the SDK host.deleteProfile path the Bots
plugin uses.
Regression tests cover local and source-scoped routes; verified red
with the cleanup disabled.
The salvaged proactive validation (#53307/#76896) rejects the
b'\x89PNG...' + zero-padding fixtures test_vision_tools.py used.
Replace all 11 sites with a real 1x1 PNG (+ ignored trailing padding
for size-sensitive tests), and pin _resize_image_for_vision passthrough
in the size-limit test so it exercises rejection, not resize recovery.
With both salvaged validators in place, the resolver-boundary verify()
(#53307) catches a truncated PNG before the full-decode gate (#76896)
runs. Either error message means the bytes stayed out of history.
Pillow is an optional dependency in this codebase (every other PIL use in
tools/vision_tools.py imports lazily and falls back). Both salvaged
validators now distinguish 'PIL missing' (pass through, header-only
sniff) from 'decode failed' (reject), so a Pillow-less install keeps
working instead of rejecting every PNG.
Follow-up to salvaged #53307 (@CannibalKush) and #76896 (@HaiyiMei).
MiniMax's Anthropic-compatible endpoint rejects an oversized native image
part with "media exceeds size limit: max 10485760 bytes (2013)" — no
occurrence of the word "image", so none of _IMAGE_TOO_LARGE_PATTERNS
matched. The 400 fell through to _REQUEST_VALIDATION_PATTERNS (the body
is type: invalid_request_error) and classified as format_error /
non-retryable.
That skipped the image-shrink recovery in conversation_loop, which is
gated on FailoverReason.image_too_large. Because the oversized part is
already baked into history as a tool_result image block, and the context
compressor rewrites text but not image data, every later turn re-sent the
same bytes and failed identically — the session stayed dead until the
user forked it.
Match on the "media" fragment, mirroring the existing "image exceeds"
entry so reworded vendor variants are caught too. A non-image media
rejection routed here is safe: the shrink pass finds no image parts,
returns False, and the caller surfaces the original error unchanged.
Fixes#76039
Second review round: honoring base64Encoded recursively let any nested
dict spoof {"base64Encoded": true, "data": "<secret>"} past the redactor
(Runtime.evaluate returns arbitrary by-value JSON), and two carriers were
missed entirely — Network.streamResourceContent returns unflagged binary
bufferedData, and Network.getRequestPostData's postData was not covered.
Replace the ambient field sets with per-method exact result-path specs:
_CDP_ALWAYS_BINARY_PATHS for declared-binary paths (screenshots, PDFs,
streamResourceContent, beginFrame screenshotData, the nested
CacheStorage.requestCachedResponse.response.body) and
_CDP_FLAGGED_BINARY_PATHS for paths whose carrier object's base64Encoded
sibling gates the exemption (Network/Fetch.getResponseBody body, IO.read
data, getRequestPostData postData). Path suffixes propagate only into the
matching subtree, so base64Encoded is type information solely on trusted
carrier objects — never ambient trust in nested JSON.
Architecture-review follow-up: the method-scoped binary_payload flag skipped
redaction for every string anywhere in the result of the two listed methods,
and the same corruption stayed reachable through Network.getResponseBody /
Fetch.getResponseBody / IO.read / Network.streamResourceContent. Make the
exemption field-scoped instead: an explicit schema exempts exactly
Page.captureScreenshot.result.data and Page.printToPDF.result.data (carriers
with no flag of their own), and any dict whose base64Encoded sibling is
exactly True exempts its body/data/bufferedData string (the protocol's own
discriminator — text bodies with base64Encoded: false stay redacted). Every
other string in every result keeps full secret redaction.
_redact_cdp_output applied redact_sensitive_text(force=True) to every string
in CDP results, including the base64 screenshot/PDF payload of
Page.captureScreenshot and Page.printToPDF. The Fernet pattern (gAAAA + base64
alphabet) matches arbitrary spans inside such payloads wherever gAAAA follows a +
or /, collapsing them to first6...last4: decoded PNGs came out corrupt (valid header,
CRC failures mid-IDAT, no IEND), the persisted full copies in tool_result_storage
were redacted too, and vision_analyze then embedded corrupt images that the provider
rejected with 400 invalid_image - killing resume sessions with a misleading provider
error. Skip redaction for the two binary-payload methods: the payload is binary,
not free text, so there is no secret to protect there. Every other method keeps
full redaction.
_runtime_model_config merges the agent's current identity onto the row's
existing model_config JSON. For model and provider it only SET the key
when the agent attribute was truthy, while base_url/api_mode/
reasoning_config/service_tier already deleted stale values when falsy.
When an agent rebuilt with an empty provider (inheriting the profile
default) was persisted, the previous provider/endpoint survived in
model_config while _persist_live_session_runtime updated the model
column separately. Resume then read the fresh model from the column but
the STALE provider from model_config, silently routing the resumed chat
to the wrong endpoint (e.g. a VeniceAI/empero route under a model that
should run on the profile default).
Apply the same delete-on-falsy rule to model and provider, mirroring the
or-None deletion the CLI path (_persist_model_switch_to_session) already
uses, so a stale session state can never survive into a resume override.
Existing desynced rows self-heal on the next live metadata persist.
Adds regression tests: merge drops stale provider/model when the agent
attribute is falsy, a truthy provider overwrites the stale value, resume
overrides fall back to the billing provider instead of the stale
endpoint, a real-DB round trip heals an already-desynced row, and a
first write (existing=None) reflects only the agent's current identity.
Treat the ChatGPT Codex invalid image-data 400 as an image rejection so Hermes strips image parts and retries text-only instead of aborting the session. Add coverage for the exact error wording.
Truncated or corrupt image bytes baked into immutable conversation history
get re-sent on every retry. Kimi/Moonshot reject them with HTTP 400
'prepare image failed ... failed to decode image: invalid or unsupported
image format', which was missing from _IMAGE_REJECTION_PHRASES, so the
turn exhausted retries and wedged the session instead of stripping the
images and recovering.
Adds the phrase to the recovery list plus a regression test mirroring the
exact Kimi error body. Complements PR #76896 (proactive full-decode
validation in vision_tools) with reactive recovery for already-poisoned
sessions. Fixes#76884.
The observed wire error — 'Downloaded response does not contain a valid JPG, PNG, WebP, or ICO image.' — has no match in _IMAGE_CORRUPT_PATTERNS, so it falls through to the non-retryable 400 handler and the session replays the same image parts into the same 400 until /new.
Adds the full observed sentence to the pattern list. Deliberately not the shorter prefixes: a bare 'downloaded response does not contain a valid' also matches non-image 400s, and a negative test now pins that a downloaded-response certificate 400 keeps falling through to the existing handler. Covered on both the 400 path and the message-only path.
Reported by ryuhaneul in #69078.
The image_corrupt recovery stripped images from canonical messages, not
just the retry payload — a transient provider rejection (xAI 'Invalid
PNG image.') permanently erased history, breaking the copy-on-write
contract (e762a5a473). Strip only the per-call api_messages copy (its
rows are shallow copies; the strip replaces content instead of mutating
the shared parts list) and pin history isolation with two regressions
(#69104 sweeper review).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The prior commit removed the too-blunt generic fallback (any
non-retryable 400 with image parts present -> strip and retry), but
nothing in the test suite would fail if someone brought it back --
every existing test either targets a recognized image_corrupt wording
or has no image content in the request at all.
Add test_unrelated_400_with_image_parts_does_not_strip_or_retry:
same image-bearing setup as the positive recovery test, but the 400 is
an unrelated unsupported-parameter rejection with no image wording.
Asserts exactly one provider call (no retry), the error surfaces to
the caller, and -- the regression guard -- the single call's messages
still carry the image_url part. A reintroduced generic fallback would
strip it before surfacing this unrelated error, which is exactly the
P1 that got this design reverted in the first place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review (Sol xhigh) on the prior commit found a P1: the generic strip-
and-retry fallback ("any non-retryable 400 with image parts present
strips and retries") was too blunt. It couldn't tell an actual
image-corruption 400 apart from an unrelated one — bad tool schema,
unsupported parameter, billing, content policy — that merely happened
to carry image parts in the request. Any of those would silently erase
vision history and retry the still-invalid request, degrading sessions
that were never bricked in the first place. That's worse than the bug
it was meant to fix.
Revert the generic fallback (agent/conversation_loop.py). Keep only
the classifier-routed path: FailoverReason.image_corrupt +
_IMAGE_CORRUPT_PATTERNS, checked before _IMAGE_TOO_LARGE_PATTERNS
because shrinking corrupt bytes can't repair them. Corrupt-image
wordings still route to strip-and-retry; everything else falls through
to normal (non-retryable) handling as before. Add xAI's second wire
wording for the same corruption class ("base64 string of provided
image cannot be decoded", returned on unaligned truncation vs "Invalid
PNG image." on aligned truncation) and a compound-message test pinning
that image_corrupt wins when a body matches both pattern lists.
Drop TurnRetryState.stripped_images_this_turn. It's unnecessary now
that only one branch is left: the branch already only retries when
_strip_images_from_messages reports it removed something, and that
helper strips every image part from the request in one pass — so a
second corrupt-image hit on the retried (now text-only) request has
nothing left to strip and falls through on its own. No separate
one-shot flag needed.
Add a run_conversation integration test at the sequenced-provider
layer: corrupt 400 on attempt 1, strip, retry succeeds on attempt 2,
with explicit before/after assertions on the outgoing image_url part.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: paultaki <paultaki@users.noreply.github.com>
The permanent-brick class in #69078: xAI returns 'Invalid PNG image'
when a re-serialized image part in replayed history becomes
undecodable. The existing image-error patterns cover only Anthropic
'exceeds max dimension' wordings and 'model does not support images'
strings, so the classifier lands on a generic non-retryable 400 and
neither the shrink path nor the strip path fires. Every subsequent
turn (even bare text) fails identically because the poison stays in
history — the session is permanently wedged until deleted.
Two recovery layers, deliberately separate:
- Semantic split: new FailoverReason.image_corrupt with
_IMAGE_CORRUPT_PATTERNS ('invalid png image' / 'invalid jpeg image'),
checked BEFORE _IMAGE_TOO_LARGE_PATTERNS in both _classify_400 and
_classify_by_message. Corrupt bytes route to strip-and-retry, never
to the shrink path (shrinking corrupt bytes cannot help).
- Generic fallback: any non-retryable 400 whose outgoing messages
still contain image parts gets one strip-and-retry via the existing
_strip_images_from_messages helper, guarded by a new
stripped_images_this_turn one-shot flag on TurnRetryState. This
un-bricks the session for any current or future provider wording
without adding another pattern list to maintain.
Item 3 from the report (multimodal-part integrity across FTS
persistence + compaction handoff) is a separate investigation and
remains follow-up work.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: paultaki <paultaki@users.noreply.github.com>
Follow-up to @AIalliAI's #60750 commits: with python_environ_get_secret
downgraded to medium, the high-severity python_os_environ pattern still
fired on the same os.environ.get("...KEY") line, re-escalating the verdict
the downgrade intended to avoid. Exempt every .get() form (non-secret =
config read; secret-shaped = scored medium by the dedicated pattern), and
apply the same medium grade to the sibling os.getenv() secret pattern —
same shape, same rationale (#60709 point 2).
The original ^(?!\s*#) prefix only skipped full-line comments starting
with '#'. An inline comment like:
cfg = environ.get('HOME') # os.environ available
still triggered python_os_environ because the regex matched the code part
before the '#'.
Two complementary fixes:
1. Replace ^(?!\s*#) with ^[^#\n]* in the regex — this rejects any line
where a '#' comment marker appears anywhere before os.environ.
2. Add _compute_docstring_lines() — a state machine that pre-computes
lines inside triple-quoted strings (docstrings) and skips them during
pattern matching. Also handles single-line self-contained docstrings.
6 new regression tests covering: inline comments, multi-line docstrings,
single-line docstrings, full-line comments, and a verification that real
bare dict(os.environ) code still triggers. All 85 tests pass.
Five targeted fixes for #60709 (reported by @mvanhorn):
1. ruby_env_secret: scope ENV[] to case-sensitive Ruby constant
((?-i:ENV)) — no longer matches Python env[key] dict access.
2. python_environ_get_secret: downgrade critical→medium — reading
an API key via os.environ.get() is normal auth, not exfiltration.
3. python_os_environ: skip comment lines with ^(?!\s*#) — no longer
flags os.environ references in docstrings or code comments.
4. deception_hide: downgrade critical→high + negative lookahead for
UX guidance context (unless/except/until/confirm/diagnose/verify).
5. oversized_skill: downgrade high→low + raise cap 1MB→5MB — large
skills are legitimate; structural size is informational only.
All 80 existing tests pass. 6 new verification tests added for each fix.
Review-fold from the 3-angle simplify pass:
- sed -Ei / -iE / --in-place now match the shell-critical tier (the
bare '\s-i\b' token missed combined short flags and the GNU long
form); read-only sed stays unflagged. Regression tests added.
- agent_config_contract joins plugin_guard's CODE_EXEMPT_PATTERN_IDS:
content-contract prose in plugin code files (docstrings/comments)
is the same false-positive class the existing agent_config_mod
exemption suppresses. Doc/config files keep the full pattern set.
Efficiency reviewer: 1.24x full-scan cost (+3.4ms/file, install-time
only), worst-case adversarial line 55us — no ReDoS exposure.
Follow-up hardening on top of #92249's tiered scoring:
- Shell-critical tier now also catches tee, and cp/mv with the config
file in destination position (cp/mv reads and .bak backups excluded).
A single '>' redirect must be preceded by a word/quote character so
markdown blockquotes and '->' arrows no longer match.
- Prose tier catches mid-line imperatives behind directive markers
('you must modify...', 'please update...', 'make sure to append...'),
which previously bypassed the line-start anchor.
- Prose instructions aimed at AGENT config files score critical again:
project-skill quarantine acts only on 'dangerous', so high/caution
silently converted 'quarantined' into 'allowed' for exactly the
sentence shape persistence attacks use (concern raised in #88952).
Hermes/other-agent config prose stays high/caution (setup docs
legitimately instruct config.yaml edits).
- New content-contract tier ('AGENTS.md should contain ...') at
high/caution — the shape is shared by authoring guides and attacks.
- .claude/settings and .codex/config gain the same shell-critical tier.
Verified against a 595-skill corpus: 0 skills blocked by these tiers
(main blocked 44 legitimate ones), all mattpocock repro skills from
#92021 install, and 20/20 attack corpus lines keep their verdicts.
Enough1122's review on #92249 asked whether language write APIs
(Python open('w')/write_text/os.replace/shutil, Node fs.writeFileSync/
appendFile) are covered by the agent-config persistence tiers. They are
not: those tiers score shell redirection, sed -i, and imperative prose
only; language-API calls surface just the informational *_ref finding.
Static regexes cannot tie a dynamically-built path to the config-file
destination without executing the skill, so this is a documented scope
cut rather than missing coverage — runtime install gates remain the
backstop. Scoring behavior is unchanged, so SCANNER_VERSION stays at
skills-guard-v2 and cached verdicts remain valid.
Refs #92249
Under #92021 the scanner no longer emits critical agent_config_mod /
hermes_config_mod findings for bare mentions - the migration script's
legitimate references now score as informational _ref findings and the
verdict is "safe" with zero modification-intent findings. Tighten the
assertion accordingly (verdict must be safe, not merely non-dangerous)
and swap the known-false-positive set to the new _ref ids.
The skills-guard-v1 scanner flagged ANY mention of AGENTS.md / CLAUDE.md /
.cursorrules / .clinerules as critical/persistence. Any critical finding
forces a dangerous verdict, and community installs cannot be overridden
with --force — so legitimate meta-skills that merely DISCUSS agent config
files (authoring guides, setup docs, cross-references) were permanently
blocked. Three popular community skills were hit in the wild.
skills-guard-v2 scores the persistence category in three tiers:
- Mechanical persistence (shell redirection or sed -i targeting an agent
config file) stays critical -> dangerous. An unambiguous write path.
- Modification language in imperative position (verb at line/bullet start
within 80 chars of the filename) is high -> caution. Regexes cannot
separate "Edit AGENTS.md to inject instructions" from descriptive prose,
but imperative verbs are the shape real instructions take. Caution keeps
the install confirmable instead of irreversibly blocked.
- Bare references drop to low/informational for auditability without
driving the verdict.
The verb-proximity shape matches the existing convention in
tools/threat_patterns.py, and the tiering mirrors how allowed_tools_field
was already handled. The pattern id agent_config_mod is preserved so
plugin_guard.CODE_EXEMPT_PATTERN_IDS stays valid; hermes_config_mod /
other_agent_config get parallel _shell / _ref splits fixing the whole bug
class. SCANNER_VERSION bumps to v2 so cached v1 dangerous verdicts are
invalidated and re-scanned on next install attempt.
* fix(execute_code): limits line teaches spillover — big stdout is saved, not lost (follow-up to #97043)
* fix(execute_code): drop the editorializing tail from the limits line (maintainer review)
fangliquanflq