* fix(skills): load SKILL.md saved as UTF-8 with a BOM
The front-matter anchor began at ^---, so the U+FEFF that Windows Notepad and
PowerShell's `Set-Content -Encoding UTF8` prepend made the match fail. parse_skill_file
then returned None and the catalog builder dropped the skill without logging, leaving an
author with a valid SKILL.md that simply never appears. split_skill_markdown reported
"No YAML frontmatter found" for the same file at install time.
Let the anchor consume one leading byte-order mark, and reuse the shared compiled pattern
in parser.py instead of the duplicated inline copy so the two cannot drift apart again.
* fix(skills): capture the description of a BOM-prefixed SKILL.md
The reader in skill_context anchored on ^---, so the byte-order mark that this change already
tolerates in the loader and validator made the match fail and build_skill_entry_metadata_from_read
capture an empty description. render_skill_context then listed the skill without the one line the
active-skill reminder shows for it.
Consume one leading byte-order mark there and pin the capture with a regression test.
or 0.5 cannot distinguish an explicit 0.0 from an unset field, so a discounted fact got the default confidence weight in both the FTS5 document and the substring fallback, while the same row indexed through FTS5Retrieval.index_fact kept its zero weight.
* fix(sandbox): resolve the async acquire lock path off the event loop
_discover_or_create_with_lock_async offloads every step of the acquire —
ensure_thread_dirs, the lock file, cache re-checks, backend discovery and
registration — except the one line that builds the lock path itself, which
resolves through Paths.base_dir and therefore runs a syscall on the loop.
The sibling anchor documented that as "a pre-existing blocking call in this
coroutine" and stubbed the path layer to get past it. With the resolution in
a worker thread the stub is no longer needed, so this removes it and pins the
behaviour with an anchor of its own.
* test(sandbox): word the offload anchor against the state after the stub removal
* test(sandbox): pin that the gate still catches an inline thread_dir call
The lock-path anchor only asserts execution reached the lock file, so it would
stay green if the gate stopped instrumenting the syscalls behind Path.resolve()
and the inline call were restored. Add a meta-check next to the store probe's:
call thread_dir() directly on the event loop under the same isolation and
assert BlockingError.
* fix(search): coerce DDG result limits from environment variables
* fix(search): fall back for invalid DDG result limits
---------
Co-authored-by: kwsYegar <331533331+kwsYegar@users.noreply.github.com>
* feat(storage): add a content-addressed blob store contract (#4189 item 2)
Addresses the review on the previous head:
* the retention/dedup claim in the doc now states the deletion rule (liveness
from the durable references) instead of promising refcount-free deletion, and
the sidecar records the first writer as advisory provenance only;
* one validate_blob_kind() owns the kind grammar, so the message a producer sees
is the pattern that is enforced;
* adds the missing storage/AGENTS.md (registry entry included) that both
docstrings already pointed at;
* drops the dead _import_backend_module() by routing the folder scan through it;
* exports BlobNotConfiguredError from the package;
* renames _KIND_MAX_BYTES to _MAX_BLOB_BYTES (it caps one put, not a kind);
* delete failures raise the neutral BlobStoreError instead of BlobReadError.
Rebased onto current main so the guidance-budget test matches upstream.
Signed-off-by: zeng-bohan <zengbh1@gmail.com>
* fix(storage): restore the config module and backend packages dropped by the squash
The force-push to e72b435 silently lost four items the surviving code
imports, breaking `import deerflow.storage` and the test suite at
collection:
- deerflow/config/blob_storage_config.py (BlobStorageConfig +
get/set_blob_storage_config + load_blob_storage_config_from_dict,
mirroring memory_config's host-shared schema and hot-reload pattern)
- app_config.py wiring: the blob_storage AppConfig field, the import,
and the _apply_singleton_configs -> load_blob_storage_config_from_dict
call (memory stayed wired; blob did not)
- storage/backends/__init__.py (drop-in contract docstring)
- storage/backends/local_fs/__init__.py (LocalFsBlobStore re-export +
STORE_CLASS for _scan_backends)
Verified: pytest tests/test_blob_store.py tests/test_app_config_reload.py
tests/test_app_config_name_indexes.py -> 67 passed; ruff format/check
clean on all four files.
* fix(storage): name the failing digest check; document the 64 MiB per-put cap
- get_bytes: split the corruption message into size mismatch (truncation /
partial write) vs digest mismatch (same-size bit rot, reporting what the
content hashes to vs what the ref names) — the combined message reported
'got 1024 bytes, expected 1024' on same-size corruption and sent the
debugger in the wrong direction.
- docs/blob-storage.md + storage/AGENTS.md: state the _MAX_BLOB_BYTES cap
(64 MiB) and that it is backend-level defense-in-depth rather than part
of the BlobStore contract, so the externalized-tool-results follow-up
sees the limit before the first production put fails.
* test(storage): pin both get_bytes corruption branches separately
The pre-split test corrupted the blob with different-size bytes, so only
the size-mismatch branch ever ran and neither new message was asserted.
Now: different-size corruption asserts 'size mismatch', and a same-size
reversal (bit rot) asserts 'digest mismatch' — the branch the split was
actually about.
* docs(config): document blob_storage in config.example.yaml
Add the blob_storage section (enabled/backend/backend_config) mirroring the
memory section's shape, matching the Configuration section of
backend/docs/blob-storage.md, and bump config_version 46 -> 47 per the
example-file convention for schema additions. Default state is unchanged:
enabled: false, purely opt-in.
* fix(helm): align chart config version with example
* fix(storage): refresh blob store on config changes
---------
Signed-off-by: zeng-bohan <zengbh1@gmail.com>
Co-authored-by: zengbohan1 <310902929+zengbohan1@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Co-authored-by: zeng-bohan <310902929+zeng-bohan@users.noreply.github.com>
* fix(sandbox): enforce execution authorization for image reads
* refactor(sandbox): share image injection authorization plan
* docs(agents): trim image middleware guidance to fit CI budget
---------
Co-authored-by: YxinMiracle <“939157765@qq.com”>
* docs(extensions): add an eleven-page extension developer manual
Add a harness/extensions/ section in English and Chinese documenting the
deerflow-extension-api 0.2.3 contract:
index when to write an extension vs a tool, MCP server or
skill; contribution kinds; loading; failure and
trust model; versioning
quick-start build, test, install and remove a working extension
runtime load sequence, API version rules, scopes and
ExtensionData, fail-open and cancellation, diagnostics
middleware placements and where each lands, scope, ordering,
observe-only wrap hooks, failure isolation
observers task lifecycle, system model calls, agent assembly
and fingerprint, context compaction
services-and-routes service lifecycle, router rejection rules, auth,
principals
run-evidence service and request-scoped readers (#5727), cursor
semantics, deletions, redaction, store differences
plugins experimental full-stack plugins, inline modules and
manifest-based browser assets (#5685)
operations extension manager CLI, sources, upgrade, rollback,
Docker, Helm, recovery
troubleshooting indexed by the exact log and error strings
reference every public name and the contract version history
Also fix stale descriptions: the contribution kinds listed in AGENTS.md
and deerflow/extensions/AGENTS.md (now eight, including plugins), the
run-evidence redaction claim (only auth_token is removed), and the plugin
mount return value in docs/full-stack-plugins.md ({ dispose }, not a
function). Changelog entries added in both languages.
* docs(agents): trim root AGENTS.md to keep inherited chains under the size limit
* docs(extensions): scope the manual to the 0.2.1 contract
Plugins and request-scoped run evidence are not in 2.1.x-dev yet; they
move to a stacked follow-up so this change can be cherry-picked.
* docs(extensions): cover full-stack plugins and request-scoped run evidence
Brings the extension manual up to deerflow-extension-api 0.2.3. Stacked on
the 0.2.1 manual so that part can be cherry-picked to 2.1.x-dev alone.
* docs(extensions): clarify PAT rejection on contributed routes
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* docs(extensions): add an eleven-page extension developer manual
Add a harness/extensions/ section in English and Chinese documenting the
deerflow-extension-api 0.2.3 contract:
index when to write an extension vs a tool, MCP server or
skill; contribution kinds; loading; failure and
trust model; versioning
quick-start build, test, install and remove a working extension
runtime load sequence, API version rules, scopes and
ExtensionData, fail-open and cancellation, diagnostics
middleware placements and where each lands, scope, ordering,
observe-only wrap hooks, failure isolation
observers task lifecycle, system model calls, agent assembly
and fingerprint, context compaction
services-and-routes service lifecycle, router rejection rules, auth,
principals
run-evidence service and request-scoped readers (#5727), cursor
semantics, deletions, redaction, store differences
plugins experimental full-stack plugins, inline modules and
manifest-based browser assets (#5685)
operations extension manager CLI, sources, upgrade, rollback,
Docker, Helm, recovery
troubleshooting indexed by the exact log and error strings
reference every public name and the contract version history
Also fix stale descriptions: the contribution kinds listed in AGENTS.md
and deerflow/extensions/AGENTS.md (now eight, including plugins), the
run-evidence redaction claim (only auth_token is removed), and the plugin
mount return value in docs/full-stack-plugins.md ({ dispose }, not a
function). Changelog entries added in both languages.
* docs(agents): trim root AGENTS.md to keep inherited chains under the size limit
* docs(extensions): scope the manual to the 0.2.1 contract
Plugins and request-scoped run evidence are not in 2.1.x-dev yet; they
move to a stacked follow-up so this change can be cherry-picked.
* docs(extensions): clarify PAT rejection on contributed routes
* feat(memory): add cognitiveStyle for collaboration preferences
Extend cross-session memory with user.cognitiveStyle and cognitive facts,
backward-compatible normalization, prompt injection as Thinking Style, and
settings UI. Add design notes and unit tests.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(memory): normalize legacy memory imports on frontend
Add normalizeMemoryPayload aligned with backend normalize_memory_data,
use it for Settings import and API reads, and add unit/regression tests.
Document schema-evolution steps and sync FACT_EXTRACTION_PROMPT cognitive category.
* fix(memory): harden legacy memory normalization
* fix(memory): keep import envelope validation strict
* fix(memory): keep unknown fields through frontend import normalization
normalizeMemoryPayload rebuilt the memory document from a section whitelist,
so every key it did not name was dropped. Because readMemoryResponse routes
all memory API responses through it, the loss showed up on read, export, and
import alike: revision vanished, and any future backend field would silently
break lossless export/import.
The Gateway models are extra="allow" and deermem normalizes additively, which
left the frontend as the only lossy hop in the chain. Spread before overriding
at each nesting level so normalization only validates and defaults
contract-owned fields, leaving the envelope check as strict as before.
Covered by a unit test on the strict import path, two backend tests for
extension preservation and caller immutability, and an E2E round-trip that
asserts on the captured import request body.
* docs(memory): state the normalization invariant that actually holds
The docstring cited `display` as a backend-driven field alongside
`revision`, but #4546 was closed unmerged and no such field exists in the
contract. It also promised lossless export/import, which the Gateway's
`extra="ignore"` models preclude for any undeclared field.
Replace both with the invariant the code is really defending: the frontend
must not be narrower than the Gateway contract, using the declared
top-level `revision` as the concrete example.
* fix(memory): preserve legacy migration and reject lossy imports
* fix(memory): align legacy fact normalization defaults
* test(memory): use authenticated app for import regression
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(authz): cover create/run/memory/agent routes with permission checks; scope USER.md per user
Route permissions: the AuthorizationProvider model only applied to routes
carrying @require_permission, so POST /api/threads, /api/threads/search,
POST /api/runs/stream and /api/runs/wait (runs:create), every /api/memory
route and the custom-agent routes ran with authentication only — a
provider configured to deny threads:write/runs:create/... could not
enforce those decisions. Add the missing decorators (threads:write,
threads:read, runs:create, memory:read/write, agents:read/write) and
register the new permission names in authz.Permissions. No owner checks
are added: per-user data scoping stays in the repository layer.
User profile: GET/PUT /api/user-profile read and wrote a single global
{base_dir}/USER.md, so in a multi-user deployment any authenticated user
could overwrite the prompt context injected for everyone else (and read
it). Scope the file to the caller's bucket
({base_dir}/users/{user_id}/USER.md) like user-scoped skills; no other
consumer of the old global path exists in the tree (verified by grep and
the updated tests). test_put_user_profile asserted the wrong effective
user under the autouse conftest user fixture; fixed to test-user-autouse.
* fix(authz): bind positional args in require_permission; migrate legacy USER.md
Review follow-up on #4989 (willem-bd):
- require_permission's wrapper only looked for request in keyword
arguments, so direct positional calls like
create_thread(body, request) made the injected keyword stub collide
with the positional request (TypeError: multiple values). The wrapper
now binds the wrapped signature via inspect.signature().bind() and
honors a positionally-passed request (and thread_id) instead of
assuming kwargs; the test-stub injection only fires when request is
absent everywhere. The two positional callers in
tests/test_threads_router.py pass again (8/8 channel tests).
- migrate_user_isolation.py now claims the legacy global USER.md for
--user-id (default 'default') like the other unowned legacy
artifacts; without it, upgrading installs with an existing profile
would read content: null and later strand the old file beside the
new per-user one. Conflict handling mirrors migrate_memory (rename to
USER.legacy.md).
* style(scripts): keep migrate_user_isolation help within the line budget
* test(authz): give internal-request stubs realistic auth fields
The create_thread permission wrapper added in this PR authenticates
the direct calls in the internal-owner tests; their SimpleNamespace
requests had state.user but no auth_source and no cookies, so
get_current_user_from_request fell through to request.cookies.get and
raised AttributeError (verified introduced by this branch: both tests
pass on the base commit).
The stubs now carry cookies={} and
state.auth_source=AUTH_SOURCE_INTERNAL, which is exactly what
AuthMiddleware stamps on real internal requests, so state.user is
honored without the JWT path. All 80 tests in the file pass.
* fix(pat): keep memory/agent permissions out of PAT scopes by design
The route permissions exist (they guard the memory/agent routers) but PATs
govern the thread/run lifecycle only: _PAT_ROUTE_RULES default-denies those
routers for PAT callers regardless of scopes. The alignment invariant
becomes a subset check plus a pinned exclusion, and pat.py documents that
opening these scopes is a product decision requiring three synchronized
changes.
* test/docs: cover migrate_user_profile; drop the unclaimed USER.md injection wording
Five-scenario test class mirroring the sibling migration steps (move,
conflict-rename, noop, both under dry-run). The GET/PUT descriptions no
longer say USER.md is 'injected into agents' — nothing at this head consumes
it for prompts; the routes are storage/retrieval only. The memory AGENTS.md
migration pointer now enumerates skills/ and the global USER.md.
* docs(paths): align the USER.md layout comment with the injection disclaimer
* test(authz): align effective permission contracts
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(skillscan): keep secret literals bound by kwargs, defaults and walrus in view
#5648 replaced the line-oriented `name[:=]value` sweep with an AST walk that
only inspected Assign/AnnAssign. A keyword argument, a parameter default and a
walrus all still read as `name=value` to that sweep, so moving an embedded
credential into a call silenced a HIGH-severity rule.
* fix(skillscan): fold constant-built secret values back into the AST scan
The pre-#5648 text sweep reported `API_KEY = "sk-" + "a1b2c3d4e5f6"`,
because its value capture stopped at the first closing quote of `"sk-`
and the remainder was never examined. Splitting the quotes does not
change the bound value, but `ast.Constant` alone now requires it to:
`+`, an adjacent literal run, and a placeholder-free f-string all bind
a compile-time constant that the HIGH-severity rule no longer sees,
while the same bytes in a file Python cannot parse still trip the text
fallback. `_python_secret_literal` folds those forms, so the AST pass
stays a precision-only change for values as well as binding forms.
Runtime-composed values (`os.environ[...] + "…"`, `%`-formatting) stay
unreported, which is the precision #5648 was after.
* fix(skillscan): fold a literal chain without reaching the recursion limit
The constant fold recursed once per operand, so a concatenation of ~1000
literals raised RecursionError on input ast.parse accepts. That exception
escaped past the per-file analyzer guard in scan_skill_dir, which drops every
finding collected for the file, so the deep chain cost the file its other
findings as well. The fold is now a stack walk that collects operands in
source order for either nesting, and the file's remaining rules keep reporting.
* fix(skillscan): report credential defaults on lambda parameters
_python_secret_bindings walked parameter defaults only for FunctionDef and
AsyncFunctionDef, so a credential moved into a lambda default (positional or
keyword-only) escaped the secret-env-assignment gate even though the
line-oriented sweep this rule replaced reported it. ast.Lambda exposes the same
ast.arguments structure, so add it to the tuple and pin both spellings.
Addresses review feedback on #5691.
---------
Co-authored-by: sxh313 <sxh313@users.noreply.github.com>
_auto_create_postgres_db opens a throwaway engine against the server's
`postgres` database and disposes it in its finally, but that await was bare.
Startup cancellation landing there abandoned a half-disposed pool that nothing
else owned — the engine is a local, and the CREATE DATABASE has already
committed under AUTOCOMMIT, so the next boot does not take this path again.
Drain the dispose through the cancellation-safe await_drained() helper the
engine's own close path already uses, and pin it with a repeated-cancellation
regression.
* fix(logging): collapse space-carrying Redirecting slots (#5225 round 16)
A Redirecting slot was kept verbatim whenever a scheme matched at
position 0, but the generic absolute-URL pass it is handed to stops
both its `host` and `rest` groups at whitespace. A Location header
value carrying an interior space (legal field syntax a misbehaving
server can emit, and the shape round 13 deliberately kept matching)
therefore survived in the clear after the first space:
'Redirecting /private/x?token=Q -> https://cdn.example/other page?sig=LeakedSig'
redacted to '... -> https://cdn.example/<redacted> page?sig=LeakedSig'.
The slot is now kept only when a whitespace-free absolute URL fills it,
so the pass is guaranteed to consume it whole; space-carrying slots
collapse like every other shape neither pass could cover.
* fix(logging): ask the URL pass whether it consumes a Redirecting slot whole (#5225 round 16 review)
The kept-set regex ([a-zA-Z...]*://\S+) was a stand-in for the invariant
"the generic absolute-URL pass consumes the slot whole", and two
whitespace-free absolute shapes satisfied it while the pass actually
stopped early: a quote that reads as a closing mark (https://h/a')b?sig=…)
and an empty host before the first /?# (https:///path?sig=…), which its
host group never matches. Both left the signed tail in the log.
Replace the stand-in with the check itself: run the pass's own pattern
against the slot and keep it only when the match spans it end to end, so
the rule cannot drift from the pattern's stop conditions again.
---------
Co-authored-by: sxh313 <sxh313@users.noreply.github.com>
* chore(checkpoint): bump to checkpoint 4.2.0 and drop the stale saver patch
langgraph-checkpoint 4.2.0 fixes the write dropped after a full -> delta
migration (upstream langchain-ai/langgraph#8526) and keeps its own
InMemorySaver override, so the compatibility patch in checkpoint_patches.py
could neither detect the fix nor stand down: it read langgraph's version
while InMemorySaver ships in langgraph-checkpoint, and it would have kept
shadowing the upstream implementation on 4.2.0. The postgres release 3.1.2
locates plain-value delta seeds (upstream #8535).
- floor langgraph-checkpoint>=4.2.0,<5.0 in the harness and raise the postgres
extra to >=3.1.2,<3.2; langgraph's own >=4.1.0 would otherwise let a
resolver reintroduce the bug now that the patch is gone
- delete the patch, its version guard, and the three implementation-assertion
tests; test_full_to_delta_migration_replays_on_same_thread stays as the gate
and fails on 4.1.1 without the patch (verified by a one-off unpatch run)
- langgraph stays 1.2.9 and langgraph-checkpoint-sqlite stays 3.1.1
Verification: make lint; delta checkpointer/cache/worker-resume suites green
with the migration gate passing on memory and sqlite (postgres params skip
without TEST_POSTGRES_URI). The full make test run before the packaging-pin
fix below was 13 failed / 18523 passed; 12 of those failures reproduce on main
with the same environment (git stash baseline, identical failure sets), 7 of
them because the local config.yaml runs checkpoint_channel_mode: delta. The
remaining one, the postgres-extra string pin in tests/test_checkpointer.py, is
updated here.
Upstream status for the surrounding gaps: cache parity across off/cold/warm
states, postgres pagination (langchain-ai/langgraph#8448) and parallel-superstep
replay order (#8382) are still open upstream, as is the abandoned-branch fork
fix (#8548) that the delta resume linearization in runtime/runs/worker.py works
around, so that path stays.
* docs(changelog): keep checkpoint upgrade in Unreleased
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(skills): resolve the user-scoped install scan from its own config
UserScopedSkillStorage.ainstall_skill_from_archive re-implements the parent
body to redirect the install target into the per-user custom root, and the
copy dropped the app_config argument. _scan_skill_archive_contents_or_raise
therefore fell back to the process-global get_app_config() for the static
scan, while the archive preflight it inherits still read self._app_config --
so the two gates of a single install could consult different configs.
The divergence is reachable whenever the storage outlives a config edit:
DeerFlowClient snapshots get_app_config() at construction, and the Gateway
hands its per-request get_config() to get_or_new_user_skill_storage, while
get_app_config() hot-reloads a later edit of config.yaml. With the storage's
config enabling skill_scan and the global disabling it, the preflight ran,
the content scan silently skipped, and a CRITICAL archive installed.
* fix(skills): thread app_config into the per-file LLM scan
_scan_skill_archive_contents_or_raise takes app_config and uses it for the
static gate, but never forwarded it to _scan_skill_file_or_raise, so the
per-file LLM scan fell back to the process-global get_app_config() for
skill_evolution.moderation_model_name and the model it constructs. Under
the stale-snapshot scenario the content-scan fix addresses, the two halves
of one install still read two different configs.
Raised in review of #5703 by willem-bd. The gateway's own skill-write
routes already pass app_config to scan_skill_content; this brings the
install path in line.
* feat(image-search): expose the color and license_image filters
`_search_images` accepts `color` and `license_image` and forwards them into the
`f` filter payload that `ddgs`'s duckduckgo_images engine builds
(`duckduckgo_images.py:62-79`), so the provider already declares support for
both. But `image_search_tool` never took them as parameters and never passed
them, so no caller -- model or config -- could set either one. They were dead
parameters: declared, wired, and unreachable.
Both matter for this tool's stated purpose, which is sourcing reference images
for image generation: `color` narrows results to the palette being generated,
and `license_image` filters to license-cleared results when the reference will
be redistributed.
Expose them on `image_search_tool` alongside the existing `size` /
`type_image` / `layout` filters and pass them through. `_search_images` only
forwards truthy filters, so an unset filter still produces the same request as
before.
Adds two regression tests: one asserting both filters reach the DDGS call
(red on main with `KeyError: 'color'`), one pinning that unset filters stay out.
* fix(image-search): add a usage hint to the color filter docstring
The sibling filters each carry a usage hint ("Use \"Large\" for reference
images", "Use \"photo\" for realistic references"), but `color` only listed
its options. Since this docstring is the model's parameter surface, add a
short hint so the model knows when the filter applies, and note that
"color" means full-color rather than a meta-parameter.
---------
Co-authored-by: RXQ6 <RXQ6@users.noreply.github.com>
* fix(subagents): report an explicit zero batch limit instead of defaulting it
SubagentBatchService.submit resolved max_live_items / max_running_items with
`or`, so a caller that explicitly passed 0 got the configured default
(100 / 3) persisted and the 1..N range guards never saw the value, while a
negative already failed there. Resolve the defaults on 'is None' so an
explicit 0 reaches the guard that names it.
* docs(subagents): state the >= 1 window constraint in the batch_task args
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(searxng): walk pageno so max_results above one page is honored
The SearXNG search API has no `limit` parameter: `/search` answers with one
page of results (the instance's `results_per_page`, 10 by default) and
ignores a limit it is handed. The client sent `limit=max_results` anyway and
hardcoded `pageno=1`, so any configured `max_results` larger than a page was
silently truncated to whatever the first page held -- an instance could not
tell a truncated response from a complete one.
Collect results by walking `pageno` until `max_results` is reached, a page
adds nothing new, or a page comes back empty. The walk is capped at
`_MAX_PAGES` so an unexpectedly large `max_results` cannot fan out into
unbounded requests. The unsupported `limit` parameter is no longer sent.
Adds four regression tests covering cross-page collection, the single-request
fast path, dedup-driven early stop, and the absence of `limit`.
* Remove redundant condition for limit check
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: RXQ6 <RXQ6@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(tenki): honour start_line and end_line in read_file
The base Sandbox contract declares read_file(path, start_line, end_line),
and the tools layer passes both keyword arguments on every ranged read —
including the continuation path that a truncated read names with its
start_line marker. TenkiSandbox.read_file accepted only path, so any
ranged read through a Tenki sandbox raised TypeError and surfaced as
"Unexpected error reading file"; a truncated read could not be continued.
Slice the text the way the other providers do (e2b, opensandbox,
boxlite): the full text when no range is given, otherwise the selected
lines joined with newlines.
* fix(tenki): clamp a negative start_line and pin the out-of-range read contract
Mirror LocalSandbox.read_file and clamp the start line to at least 1 so a
negative start_line cannot wrap around through Python's negative-index
slicing. Also extend the ranged-read test with the two boundary cases the
tools layer depends on: a start past EOF returns an empty string, and a
negative start reads from the first line.
* fix(tenki): clamp a negative end_line in read_file
Mirror LocalSandbox.read_file for the symmetric range boundary: clamp a
negative end_line to zero so Python's negative-index slicing cannot silently
drop the last line. Pin the empty result in the existing ranged-read test.
* fix(channels): fix Telegram inbound file download and sandbox readability
- Shut down the download Bot on the Telegram loop (Bot.shutdown(), which
closes its HTTPX clients) before the loop stops, instead of the
non-existent Bot.session.close().
- Grant group/other read on channel-downloaded uploads so the non-root
AIO/Docker sandbox process can read the root-written 0o600 file.
- Apply the sandbox permission change with os.fchmod on a descriptor
opened with O_NOFOLLOW (validated as a regular file via fstat), bound
to the validated upload inode, so a symlink swapped in after lstat
cannot redirect the chmod to a target outside the uploads directory.
The open also uses O_NONBLOCK so a sandbox-swapped FIFO cannot block the
read-only open before the regular-file check (matching the existing
open_upload_file_no_symlink convention).
Centralized in a shared apply_upload_sandbox_permits helper reused by
the channel inbound path and the HTTP upload readable/writable helpers.
- Surface the download failure cause chain in logs with the Bot API URL
masked: the configured token is redacted and both URL forms are
collapsed, covering the file download URL (/file/bot<token>/...) and
the method URLs (/bot<token>/getMe, /bot<token>/getFile).
- Migrate the existing receive_file tests onto the download Bot and add
coverage for _get_download_bot (loop-bound creation + caching, cleanup
on init failure), download-bot routing over the application bot,
real-Bot shutdown closing both HTTPX clients, receive_file
timeout-containment, masked cause-chain logging (file and method URL
forms), and inbound-file sandbox perms (including the swap-after-lstat
symlink regression).
* fix(uploads): surface sandbox permission failures
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(sandbox): bound remaining shell session cleanup requests
release_command_scope(), close()'s scoped-session drain, and close()'s recovery-session cleanup called shell.cleanup_session() without a request budget, so a stalled cleanup could hold the thread-key serializer, scoped.lock, or the sandbox lock for the SDK's default transport budget. Pass the existing _bounded_cleanup_request_options() (5s, max_retries=0) at those three sites; _cleanup_session_best_effort() and its swallow-and-log contract are unchanged, as are create_session and list_dir lifetimes.
* fix(sandbox): bound AIO list_dir with a directory deadline
Give AioSandbox.list_dir its own 60s directory deadline with a 65s no-retry
host envelope, independent of bash_command_timeout. Preserve #5634's
shell-generation selection: list_dir runs on the current recovery session once
the implicit shell is fenced, and an ambiguous list_dir outcome - transport
timeout or an ambiguous returned status - fences whichever generation actually
executed the request, dropping local recovery ownership and attempting bounded
best-effort cleanup instead of leaving that session reusable. hard_timeout
remains definite termination and keeps the targeted session reusable. Only
completed/None results are parsed, so a partial find is never returned as a
complete listing.
Session creation RPC lifetime remains out of scope.
* fix(sandbox): recover sessions after transport failures
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
web_fetch_tool parsed `wait_for_timeout_ms` with a bare int(), so any value
that is not already an int raised ValueError and aborted the whole tool:
a quoted number in YAML, or a typo like `2s`, turned every fetch into
"Error: invalid literal for int() with base 10". web_capture_tool reads the
same documented key through the tolerant `_as_int` helper two functions
above, so the two tools disagreed about the same config key.
While aligning them, web_fetch_tool now also reads
`wait_for_selector_timeout_ms` instead of hardcoding 5000, so a
`wait_for_selector` on a slow page can be given more than five seconds --
web_capture_tool already honours the key and the client has always
accepted it.
Co-authored-by: RXQ6 <RXQ6@users.noreply.github.com>
* fix(skills): install for a user keeps its custom-dir setup off the event loop
UserScopedSkillStorage.ainstall_skill_from_archive re-implemented the install
pipeline and created the per-user custom directory inline, so the await behind
POST /api/skills/install blocked the Gateway loop on os.mkdir. The base class
offloads every filesystem phase around its LLM scan — and says so — but the
override's setup step sat above that comment and escaped it, and the existing
blocking-IO anchor drove only the host-scoped class.
Offload the mkdir through the same worker thread and anchor the override itself
under the strict Blockbuster gate.
* test(blocking-io): pin per-user staging of supporting skill files
Review follow-up on #5650: the user-scoped anchor asserted only the staged
SKILL.md, so a regression that mis-stages the references/ tree under the
per-user root would pass both anchors. Assert the supporting file against the
per-user layout as well, mirroring the host-scoped twin.
* fix: rebuild todo reminders after context compaction
* chore: remove implementation plan from PR
* refactor: share todo reminder message name
* docs: keep agent guidance within CI size budgets
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(skills): render an empty allowed-tools as no tools, not as all
`_render_skill_metadata` truthiness-tested `Skill.allowed_tools`, so an
explicitly empty allowlist (`allowed-tools: []`, parsed to `()` by
`parse_allowed_tools`) rendered "Allowed tools: (all)" -- the same text an
omitted field (`None`, unrestricted) produces. `allowed_tool_names_for_skills`
distinguishes the two and strips every business tool for `()`, so the
describe_skill output contradicted the policy applied to the same skill.
* docs(skills): scope the rendered (all) to the skill's own declaration
Review note: allowed_tool_names_for_skills makes a legacy None skill
contribute no tools once any loaded skill declares allowed-tools, so
"(all)" on that line reports the frontmatter of this skill rather than
the tool set the middleware will allow in a mixed set. Say so where the
tri-state is rendered; behaviour is unchanged.
* fix(uploads): stop deleting a converted companion we cannot prove we wrote
Conversion names a document's Markdown companion after the document's
stem and falls back to a _N suffix when that name is already taken, so
the .md beside a document may belong to another document sharing the
stem, or to the user. Delete removed it anyway: uploading a.docx and
a.pdf produces a.md and a_1.md, and deleting a.pdf destroyed a.docx's
companion while orphaning a.pdf's own.
Delete now removes only the file it was asked to remove. The companion
stays listed and can be deleted on its own. Orphans are the cost of not
guessing; issue #5672 covers giving companions a provable owner, which
is what a safe cleanup needs, along with the two related readers that
still guess (the outline injected for a document and the agent's file
listing).
convertible_extensions loses its last use and is dropped from the
signature and both call sites. The gateway router keeps importing
CONVERTIBLE_EXTENSIONS for the ingestion bridge and now declares it in
__all__, where that module documents its re-exports.
* docs(changelog): note that delete keeps the converted markdown (#5673)
* fix(runtime): share one change position across an atomic thread operation
MemoryRunStore.create_thread_operation_atomic marked each row it touched
separately, so one interrupt-and-replace consumed two positions in the
(change_seq, run_id) cursor that list_changed consumers page with. The SQL
store allocates a single position for the whole set, and runtime/AGENTS.md
documents that as the contract for an atomic thread operation.
The memory store is the backend an install runs whenever no durable database
is configured, so a cursor reader could observe the interrupted row and its
replacement under two positions instead of one, ordered by run_id.
Allocate one position after the raise-only scan -- so a rejected operation
still consumes nothing -- and reuse it for the claimed rows and the new row.
* test(runtime): pin that a rejected thread operation consumes no position
create_thread_operation_atomic allocates its change position only after the
raise-only candidate scan, so a ConflictError consumes nothing -- the memory
store's counterpart of the SQL store rolling back and leaving its clock
untouched. Nothing asserted that ordering.
A consumed-but-unused position leaves no trace in the rows themselves, so a
test comparing list_changed output passes even with the allocation hoisted
above the scan. Assert instead on the position the next accepted operation
lands on: that surfaces a hoist as a gap.
* fix(sandbox): enforce AIO shell command timeouts
Map Sandbox.execute_command(timeout=T) onto the AIO legacy shell as a server-side hard_timeout=T plus a bounded host request (ceil(T+5)s, max_retries=0), preserve and render the upstream status (hard_timeout -> terminated + Exit Code: 124; no_change_timeout -> may still be running), keep no_change_timeout from preempting hard_timeout, and restrict ErrorObservation replay to completed results.
* fix(sandbox): contain ambiguous AIO command outcomes
Treat a transport timeout and statuses terminated/no_change_timeout/unknown as ambiguous: never replay the current command, fence the scoped or recovery session generation with bounded best-effort cleanup, and never adopt a replacement session for those statuses. hard_timeout and completed keep the session reusable. Align the env-bearing bash.exec path with the same contract and make its retry result authoritative.
* fix(sandbox): wire AIO command timeout from provider config
Restore bash_tool's non-local behaviour (other providers keep their own defaults) and let AioSandboxProvider read sandbox.bash_command_timeout as the sandbox's default_command_timeout at every construction site, with explicit per-call timeouts still winning. Widen the config field to float with allow_inf_nan=False and read the provider value with the module fallback so partial-config providers keep working.
* docs(sandbox): document AIO command timeout semantics
Document the provider-scoped deadline (LocalSandbox/AioSandbox/OpenSandbox wire it; other providers keep their defaults), the semver-vs-frozen-:latest image behaviour, the no-replay guarantees, and trim the sandbox guidance back under its size budget.
* docs(sandbox): scope the wedge-resistance claim to command requests
willem-bd's review on #5634 reproduced that list_dir and both create_session call sites still make unbounded SDK requests while holding the same sandbox lock, so the guide must not read as if every AIO request is bounded. State that the bounded T+5s host wait applies to command requests and name session-creation and file/list RPCs as not covered; bounding those remains a separate operation-deadline/control-plane follow-up.
* fix(sandbox): make list_dir honor the shell generation fence
list_dir previously always targeted the implicit persistent shell, so after an ambiguous command outcome fenced that generation (#5634) a listing could re-enter it. Route default-shell target selection through _ensure_default_shell_session_id so both execute_command and list_dir reuse the explicit recovery session, creating one only when the implicit shell is fenced. No list_dir deadline, hard_timeout, request budget, retry, or status semantics are added; those remain scoped to #5644.
* fix(mcp): honor configured stdio working directories
* fix(mcp): preserve defaults for empty working directories
Treat empty stdio cwd values as omitted, including unresolved environment references. Add real subprocess regressions for discovery and pooled-call defaults, plus direct connection-dictionary coverage.
A Redirecting slot stayed verbatim unless it started with "/", but the
Location field-value grammar (RFC 3986 relative-part) also admits
slash-less relative references: 'Redirecting /private/x?token=Q ->
download?sign=LeakedSig' rendered the signed query verbatim, and neither
the slot rule nor the generic absolute-URL pass (which needs a scheme)
could see it. A slot is now kept only when a scheme matches at position
0 (left for the generic pass to rewrite); everything else collapses -
slash-less relative paths, query-only and fragment-only forms,
network-path references (collapsing any userinfo they carry), and
non-hierarchical schemes such as data: URIs. The ^Redirecting prefix
anchor is unchanged, so non-URL '-> /path' arrows (sandbox mount
mappings) keep passing through untouched.
Regression table covers all relative-reference forms plus the absolute
regression anchor; mutation-verified (reverting to startswith('/') goes
red). Aligned with the fix sketched in the #5225 round-15 review thread.
* fix(skillscan): read Python secret assignments from the AST
The `secret-env-assignment` rule swept every text file with a
`name[:=]value` regex, which misreads Python syntax in two ways:
- `def __init__(self, token: Optional[str] = None):` — the captured
"value" is a type annotation, not embedded secret material.
- `api_key = os.getenv("MINIMAX_API_KEY")` — reading a secret from the
environment is this rule's own documented remediation, yet it was
reported as a hardcoded credential.
Both are HIGH severity, so they map to a review `error` and fail the
Skill Review gate. Two bundled public skills therefore failed CI on an
unchanged checkout:
- skills/public/github-deep-research/scripts/github_api.py:56
- skills/public/music-generation/scripts/generate.py:27
Python sources now go through the AST instead of the line-oriented
sweep, keeping only real literal values. The text sweep is unchanged for
config, shell, YAML, and Markdown. Annotated assignments are still
reported, and now at the literal rather than at the annotation.
Tests: `secret-env-assignment` previously had no coverage anywhere in
backend/tests. Added six tests, including a regression test that scans
every bundled public skill script. Verified red on main and green here.
* fix(skillscan): keep text coverage for unparseable Python
Reviewer feedback on #5648: when `ast.parse` failed, the rule returned no
findings at all. One syntax error -- or a NUL byte, which `ast.parse` rejects
with the same exception -- therefore silenced the HIGH-severity
`secret-env-assignment` rule for the whole file, where `main` still swept the
raw text and reported it. For a review-gate rule that is a trivial evasion.
The line-oriented sweep moves into `_scan_secret_assignments_by_text`, which
the non-Python path now calls and which `_scan_python_secret_assignments` falls
back to when the file will not parse. Parseable files keep the precise AST
semantics this change introduces; unparseable ones keep main-level coverage
instead of losing the rule entirely.
Tests: both fallback paths added (syntax error, NUL byte); both fail before this
commit and pass after.
