mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-11 14:38:38 +00:00
58 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ryker_Feng
|
f52818fe5e
|
feat(skills): export custom skill packages with revision-bound preview (#5332)
* feat(skills): export custom skill packages with revision preview * docs(gateway): keep export guidance within size budget * ci: retry checks after transient uv setup download failure * docs: focus skill export agent guidance on maintenance invariants * fix(skills): handle export disconnects and bound archive transfers * docs(gateway): remove redundant export guidance to fit merged budget * fix(skills): reset export idle deadline after transfer progress |
||
|
Wenchao An
|
0b3dadbc9b
|
feat(subagents): add acceptance checks to durable batch items (#5289)
* feat(subagents): check and persist durable batch acceptance Carry optional per-item criteria into native subagents, reuse the deterministic checker, and expose separate verdicts through item queries and exports. Preserve execution and retry semantics, renew leases during checks, and migrate existing batch rows with nullable acceptance fields. * fix(subagents): align batch acceptance normalization and sandbox admission * test(auth): include project permissions in the full-stack contract |
||
|
RongJie G
|
99367100fb
|
fix(persistence): preserve rollback across the incarnation migration (#5219)
* fix(persistence): tolerate thread incarnation migration * docs(persistence): pin forward revision contract --------- Co-authored-by: CorgiBoyG <CorgiBoyG@users.noreply.github.com> |
||
|
Jun
|
d1f1c49dcd
|
fix(workspace-changes): avoid draining metadata scans on cancellation (#5234)
* fix(workspace-changes): keep metadata cancellation responsive * test(workspace-changes): cover metadata cancellation latency * docs(workspace-changes): document cancellation ownership * style(workspace-changes): format cancellation regressions * docs(workspace-changes): remove unapproved nested guidance * fix(workspace-changes): log only real cancellation drains * style(workspace-changes): apply repository ruff format * docs(harness): record workspace scan cancellation ownership * docs: compact harness guidance below inherited size limit --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Jun
|
d7afdbf9a3
|
fix(workspace-changes): drain snapshot scan before cancellation cleanup (#5232)
* fix(workspace-changes): drain cancelled snapshot scans * test(workspace-changes): cover cancellation during snapshot scan * fix(workspace-changes): consume drained scan outcome * test(workspace-changes): keep scan cancellation regression focused * test(workspace-changes): pin cleanup ownership under recancel |
||
|
Hao Zhe
|
ec274bdedb
|
fix(memory): enforce backend read failure policy (#4726)
* fix(memory): enforce backend read failure policy * fix(memory): harden failure policy handling * fix(memory): narrow strict read handling * fix(memory): keep timeout handling off saturated executor * fix(memory): preserve legacy fail-closed timeouts |
||
|
Michael
|
eebe909ebd
|
fix(agents): make the injected current-date timezone configurable (#5154)
* fix(agents): make the injected current-date timezone configurable ## Why The date reminder injected into the lead and subagent prompts (DynamicContextMiddleware / SubagentDateContextMiddleware) was formatted with the server's local wall clock. DeerFlow containers default to UTC, so a user in Asia/Shanghai chatting in the 00:00-08:00 window was told that 'today' is the previous day - the model then reasons, plans, and date-stamps against the wrong day. ## What changed - _format_current_date() now reads the optional DEER_FLOW_DATE_TIMEZONE env var (IANA name, e.g. Asia/Shanghai) and renders the date in that zone. - Unset = unchanged server-local behavior; invalid names log a warning and fall back to server-local. - Documented the knob in config.example.yaml, the module docstring, and the DynamicContext entry in agents/middlewares/AGENTS.md. ## Surface area - [x] Agents / LangGraph - prompt-layer date context only; message shape and midnight-update behavior unchanged - [ ] Frontend UI / Backend API / Sandbox / Skills / Dependencies - [x] Default behavior change (opt-in via env var - no behavior change unless set) ## Bug fix verification - New tests: test_format_current_date_honors_configured_timezone (UTC 20:30 -> 2026-09-03 in Asia/Shanghai), test_format_current_date_defaults_to_server_local_without_env, test_format_current_date_invalid_timezone_falls_back. - Existing mocked-datetime tests pass unchanged (no env -> datetime.now() path). ## Validation - cd backend && python -m pytest tests/test_dynamic_context_middleware.py: 31 passed. - blocking_io/test_dynamic_context_middleware.py: 2 pre-existing abefore_agent failures reproduce identically on clean main (blockbuster os.listdir detection on this host); the other 2 pass. - ruff format + ruff check clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): avoid passing tz to datetime.now when no timezone is configured CI (backend-unit-tests shard 2) failed in test_tool_error_handling_middleware.py::test_subagent_chain_injects_date_without_memory_and_coalesces_for_strict_provider because its _FrozenDateTime.now() subclass override accepts no arguments, while _format_current_date() called datetime.now(None) even when DEER_FLOW_DATE_TIMEZONE was unset. - _format_current_date() now calls datetime.now() with no arguments unless a timezone is actually configured, preserving the exact legacy call shape for every datetime-subclass test fake. - The configured-zone path still calls datetime.now(tz) and converts via astimezone(tz). - Updated the no-env unit test to assert datetime.now() is called without arguments. Validation: python -m pytest tests/test_dynamic_context_middleware.py + the previously failing strict-provider test: 32 passed. ruff clean. * fix(agents): declare the effective current-date timezone in the assembly descriptor ## Why Maintainer review on the DEER_FLOW_DATE_TIMEZONE change (#5154): the knob is prompt-affecting, yet both DynamicContextMiddleware and SubagentDateContextMiddleware were invisible to the agent assembly descriptor - describe_middleware() fell back to {"probed": true} for unset, UTC, and Asia/Shanghai alike, so deployments that inject different dates shared one assembly fingerprint and release observers could not distinguish or audit the behavior change. ## What changed - Both middlewares now implement release_policy_parameters() -> dict[str, object], declaring {"current_date_timezone": <name>} as required by the module's middleware self-description contract. - The declared value is the normalized effective zone: a configured, valid DEER_FLOW_DATE_TIMEZONE is reported by its IANA key (ZoneInfo.key); otherwise the server-local zone is resolved to its IANA key when the platform exposes one and to its tzname label otherwise (fixed-offset hosts), with "UTC" as the final fallback. - Added both middlewares to _MIDDLEWARE_DECLARATIONS in backend/tests/test_middleware_release_policy.py so the existence check and the construct-and-canonical-hash check cover them. ## Verification - New tests: test_date_middlewares_declare_configured_timezone (Asia/Shanghai), test_date_middlewares_declare_utc_timezone, plus resolved-server-local assertions for the unset and invalid-env paths; both middlewares agree in every case. - cd backend && python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py: 70 passed. - Regression spot-check: tests/test_agent_assembly_descriptor.py, tests/test_tool_error_handling_middleware.py, tests/test_system_message_coalescing_middleware.py: 102 passed. - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): stabilize the declared date timezone and simplify the formatting path ## Why Follow-up review on #5154 (willem-bd). The release-policy declaration added in 884cec4b resolved the observability gap but pinned far less identity than its docstrings claimed, and the formatting path carried a production no-op. ## What changed - The declared label is now stable and unambiguous: a configured, valid DEER_FLOW_DATE_TIMEZONE is reported by its IANA key; without one, the server-local zone is resolved to a real IANA key from the TZ env var or the /etc/localtime symlink (Linux/macOS); when no key is recoverable (Windows, stripped containers) the declaration falls back to a stable `server-local(+-HH:MM)` sentinel carrying the current UTC offset. It never reports a bare abbreviation - datetime.now().astimezone() yields only a fixed-offset timezone whose tzname (e.g. CST, EST/EDT, CET/CEST) is ambiguous or DST-churns, which the assembly descriptor docstring says must not happen. - Dropped the redundant astimezone(tz) in _format_current_date(): datetime.now(tz) already returns the instant expressed in tz. The configured-zone test now fakes datetime.now(tz) semantics (the fixed instant converted into the requested zone) instead of relying on that conversion. - Documented why the knob is an env var, not a config-schema field: it is read at runtime by both date-context middlewares so an operator can point a container at another zone without mounting a config.yaml (module docstring + config.example.yaml note). - AGENTS.md: fixed the glued DynamicContext sentence (missing separator). - Added tzdata>=2025.1 to the harness runtime dependencies (with uv.lock) so ZoneInfo works on stripped containers / Windows without an OS zone database. ## Verification - New tests: test_server_local_timezone_name_reads_tz_env, test_effective_timezone_sentinel_uses_offset_when_local_zone_is_not_resolvable; reworked test_format_current_date_honors_configured_timezone to exercise the real datetime.now(tz) path. - cd backend && python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py tests/test_agent_assembly_descriptor.py tests/test_tool_error_handling_middleware.py: 140 passed. - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): offload subagent date injection off the event loop ## Why Follow-up review on #5154 (willem-bd, P2): SubagentDateContextMiddleware.abefore_agent() called _inject() directly, so enabling DEER_FLOW_DATE_TIMEZONE could synchronously read the OS timezone database (or the tzdata wheel) on a cold cache - filesystem work on the async subagent execution path whenever no assembly observer resolved the zone first. ## What changed - SubagentDateContextMiddleware.abefore_agent() now offloads the injection via asyncio.to_thread with the same bounded timeout DynamicContextMiddleware uses (issue #3402); on timeout it logs and skips the date update for that run instead of blocking the loop. - Narrowed the exception handling in _date_timezone() and the TZ-env branch of _server_local_timezone_name() to configuration-shaped failures (ZoneInfoNotFoundError / ValueError / OSError). Previously a blanket `except Exception` also swallowed BlockingError raised by the blocking-I/O regression gate, mislabeling a loop-blocking call as an invalid timezone and silently degrading to server-local - which made the new regression anchor useless. Other exceptions now propagate. ## Verification - New blocking-I/O regression anchor (backend/tests/blocking_io/test_subagent_date_context_middleware.py): drives a real create_agent graph under the strict Blockbuster gate with the knob enabled and asserts the date reminder is injected. Verified it fails (BlockingError) when the offload is reverted and passes with it in place. - python -m pytest tests/blocking_io/test_subagent_date_context_middleware.py: 1 passed. The two pre-existing os.listdir failures in tests/blocking_io/test_dynamic_context_middleware.py reproduce unchanged on this host (same as clean main). - python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py tests/test_tool_error_handling_middleware.py tests/test_agent_assembly_descriptor.py: 139 passed; the single ToolReceiptMiddleware-ordering failure reproduces with the change stashed (local extensions registry, unrelated to this PR). - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. * fix(agents): read the direct /etc/localtime symlink target for the zone key ## Why Follow-up review on #5154 (willem-bd, P2): on macOS, /etc/localtime commonly points to /var/db/timezone/zoneinfo/<zone>, but Path.resolve() follows that directory's own symlink and yields a versioned path such as /private/var/db/timezone/tz/2026c.1.0/zoneinfo/Asia/Shanghai, which matched no configured prefix. The server-local resolution then returned None and the assembly descriptor fell back to a server-local(+HH:MM) sentinel even though the IANA key was available - conflating zones that share an offset and making DST-based fingerprints unstable. ## What changed - _server_local_timezone_name() now reads the direct symlink target via os.readlink("/etc/localtime") instead of Path.resolve(), so macOS' unversioned zoneinfo path is seen as-is and its IANA key is preserved. - The zone key is taken from whatever follows the last "/zoneinfo/" segment, which also handles Apple's canonical versioned path when a direct target already carries it, and relative targets are normalized against /etc. - Removed the now-unused Path import and the fixed zoneinfo prefix tuple. ## Verification - New tests: test_server_local_timezone_name_reads_direct_macos_symlink_target, test_server_local_timezone_name_reads_apple_versioned_symlink_target, and test_server_local_timezone_name_normalizes_relative_symlink_target. - python -m pytest tests/test_dynamic_context_middleware.py tests/test_middleware_release_policy.py tests/test_agent_assembly_descriptor.py: 105 passed (75 after re-running the first two on the merged main). The blocking subagent anchor still passes; the two pre-existing os.listdir blocking failures on this host are unchanged. - ruff check + ruff format clean. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis, implementation, and regression tests produced with AI assistance; reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
9e0fbd60fa
|
fix(sandbox): isolate concurrent subagent shell sessions (#5134)
* fix(sandbox): isolate concurrent subagent shell sessions * fix(sandbox): make execution acquire idempotent * fix(sandbox): close execution lifecycle gaps * fix(sandbox): serialize retained client lifecycle * fix(sandbox): close remaining client lifecycle gaps * fix(sandbox): unwind failed client lookup * fix(sandbox): protect internal lease identities * fix(sandbox): make cancellation reconciliation durable * fix(sandbox): fence cancelled workers and IM uploads |
||
|
hataa
|
08b27aef73
|
feat(auth): make login rate-limit parameters configurable, fixes #5108 (#5110)
* feat(auth): make login rate-limit parameters configurable, fixes #5108 Add auth.local.max_login_attempts (default 5) and auth.local.lockout_seconds (default 300) so operators can tune the per-IP login throttle: raise the ceiling for shared-egress-IP offices behind proxies/NAT, or tighten it for stricter posture. Policy is live-read per call (matching the _local_registration_enabled precedent), so a config reload applies without a Gateway restart; raising the threshold mid-lockout immediately unblocks affected IPs. Review feedback addressed (willem-bd): - Only FileNotFoundError falls back to the hardcoded defaults; a malformed config propagates, mirroring _local_registration_enabled, so an operator who tightened the policy never silently gets the more permissive defaults. - _check_rate_limit looks up the record before resolving the policy, so a clean IP pays zero config reads (get_app_config re-hashes config.yaml per call and login_local is an unauthenticated async endpoint). Bumps config_version to 39 in config.example.yaml and the Helm chart (values.yaml + README example) so the chart drift check stays green. * fix(auth): reject max_login_attempts=1 and honor live lockout_seconds for active lockouts * fix(auth): close live-policy state gaps in login throttle (resurrection, count reset, broken-config verification) * fix(auth): commit evaluated lockout duration on decreases too, preventing raise-resurrection * test(auth): pin broken-config fail-closed sequence through the login route * fix(auth): sweep expired locks by stored sentence and keep policy reads off the event loop * fix(auth): re-read throttle record after the policy-resolution yield point --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
wutongyuonce
|
fb722770e4
|
fix(agents): do not hide invalid config with file fallback (#4952)
* fix(agents): do not hide invalid config with file fallback * test(agents): cover invalid on-disk config fallback * fix(agents): resolve stores off the event loop * fix(agents): distinguish missing nested config from main config * fix(agents): reject missing explicit config path * test(agents): isolate config fallback test * test(agents): isolate router blocking IO coverage * test(agents): pin malformed config.yaml parse-error propagation An unparseable config.yaml used to be swallowed by the broad except Exception and silently downgrade to FileAgentStore. The narrowed except FileNotFoundError already propagates yaml.ParserError/ScannerError; pin that contract with a real on-disk config instead of monkeypatched get_app_config. |
||
|
Aari
|
340bff1107
|
feat(mcp): manage servers from Settings (#5022)
* feat(mcp): manage servers from settings * fix(mcp): make settings updates targeted * fix(mcp): reject ambiguous masked array edits * fix(mcp): honor targeted server field deletions * fix(mcp): preserve OAuth extension secrets * fix(mcp): validate config before persistence * fix(mcp): preserve environment placeholders * fix(mcp): harden targeted configuration routes * docs: keep gateway guidance within budget * fix(mcp): protect per-tool override secrets * fix(mcp): keep disabled edits structurally safe |
||
|
Ryker_Feng
|
8d8ca506ba
|
feat(artifacts): download run files as zip (#5117)
* feat(artifacts): download run files as zip * fix(artifacts): address archive review feedback * fix(artifacts): gate unavailable archive downloads * fix(artifacts): verify archive availability * fix(artifacts): harden archive consistency * fix(artifacts): reject archive path aliases |
||
|
Zeren Wang
|
a06a6fed7e
|
feat(harness): deterministic acceptance checklist for subagent delegations (RFC #4651, layer 2) (#5109)
* feat(harness): deterministic acceptance checklist for subagent delegations (RFC #4651, layer 2) PR4 of RFC #4651: check lead-supplied acceptance_criteria in code when a subagent completes, so objectively checkable requirements can never be silently passed by a self-report. - subagents/acceptance_checks.py: deterministic leaf families — file:<path> exists|non-empty and file_written:<path> read through read_current_file_content scoped to the shared thread workspace; the read uses the sandbox-native virtual path form (the local read validator and provider mount tables resolve /mnt/user-data/... paths, not host paths); the scope decision canonicalizes with realpath on the local sandbox so workspace symlinks cannot escape into uploads; a remote provider's "Error: ..." return string is normalized to a failed check (provider-typed via is_local_sandbox); a UnicodeDecodeError marks a binary deliverable as existing and non-empty; out-of-scope paths degrade to UNVERIFIED. tests_passed:<command> anchors to a matching recorded bash execution with status=success and a test-summary shape; matching is shell-structure aware with control-flow attribution (span must end at the last segment with provable execution), negating-option values are ineligible evidence and a target negated anywhere in the command degrades the match, extra flags must be selection-preserving, extra positionals widen only after a path-scoped criterion, truncated commands degrade via command_truncated, the summary shape is read only from output attributable to the matched segment (preceding segments provably silent by invocation form), and pass shapes require a nonzero passed count. Criterion text is neutralized with neutralize_untrusted_tags before storage/rendering. Anything else renders UNVERIFIED, never silently passed. - executor: accumulate bounded bash command/output evidence per streamed chunk (merged by tool_call_id, newest-capped) so subagent summarization compacting earlier messages cannot erase a recorded execution; the recorded status is the actual shell exit status parsed from the output's exit marker (signed codes included; the remote Command exited with code N form is accepted only as the whole trimmed output), falling back to deerflow_tool_meta only when no marker exists. - sandbox providers: e2b/opensandbox/tenki/boxlite append the LocalSandbox-style "Exit Code: N" marker on nonzero exit even with non-empty output; aio propagates the SDK's structured exit_code on both exec paths the same way; local timeouts append Exit Code: 124; and _truncate_bash_output always preserves a trailing exit marker (signed included) inside its budget, with a 32-char floor raising any smaller configured limit, so the actual shell outcome always survives in the output text. - task_tool: run the checklist offloaded (asyncio.to_thread) on the completed branch, failure-isolated; stamp the verdict into result metadata and render the per-criterion section into the model-visible result text. - status contract: additive subagent_acceptance_verdict transport with read-side structural validation. - delegation ledger: entry carries the verdict and renders a compact acceptance segment; gateway strips caller-forged verdicts from both ledger entries and message metadata, like the citation verdict. - blocking-IO anchor pins the offload (teeth proven red->green); leaf read errors catch only OSError/SandboxError so unexpected errors reach the task-tool-level isolation instead of being mislabeled. * fix(harness): close acceptance evidence gaps from review (RFC #4651 PR4) - negating options: overlap with a matched criterion target is now checked by path/nodeid prefix, not exact token equality — excluding a sub-path of the criterion's selection (pytest tests --deselect tests/unit/test_auth.py) degrades to UNVERIFIED instead of holds - output attribution: any redirection token in the matched final segment makes the recorded tail non-attributable (> / >> / 2> are word characters to the parser, so redirection was invisible to the matcher) - silent-source allowlist narrowed from any *activate suffix to the */bin/activate shape - status_contract docstring: restore the shared-fixture sentence and note subagent_acceptance_verdict is deliberately outside the fixture - executor: update_bash_executions publishes [] (stream carried no bash-family calls) instead of collapsing it into None, mirroring update_tool_receipts * fix(harness): close acceptance residual gaps from re-review (RFC #4651 PR4) - tests_passed: add error outcomes to the fail shapes — "4 passed, 1 error" and pytest's "ERROR <nodeid>" short summary no longer satisfy the pass shape when the exit status is swallowed (|| true) or absent; zero-error counts stay clean. - file leaves: bound the deliverable read — a "wc -c" shell size probe answers files above 50k bytes without loading ~2x their size, honoring the host-bash kill switch and falling back to the full read on any non-integer rendering, so verdicts never get less sound. - executor: record the exit marker text as status_marker on harvested bash evidence; the leaf detail now reports the marker actually seen instead of asserting a failure indistinguishable from the command's own trailing text. - extend the blocking-IO anchor to drive the probe branch inside the offload; teeth re-verified red->green. * fix(harness): close acceptance forgery and bound gaps from P2 re-review (RFC #4651 PR4) - file leaves: never read unbounded — size is established first (os.stat on the validated local host path, so the host-bash-disabled configuration needs no shell; a guarded wc -c on remote providers that renders missing/unreadable in its own words). Above the 50k cap the leaf answers from the size alone, at/below it the full read runs, and an unestablishable size degrades to UNVERIFIED instead of an unlimited fallback read. - output attribution: source/. prefixes are never provably silent — a crafted */bin/activate path shape says nothing about what the script prints, so sourced segments can no longer lend a passing summary. - executable identity: an explicitly path-spelled criterion now requires the same normalized executable path; the basename rule stays only for deliberately bare criterion commands. * fix(harness): run acceptance size probe outside subagent-controlled state (RFC #4651 PR4) - remote probe no longer runs in the sandbox's persistent shell: a fresh env -i /bin/sh with absolute-path stat/realpath (poisoned functions, aliases, PATH, exported functions, IFS, locale cannot steer it), plus a marker env routing AIO onto a fresh per-call bash.exec session. - metadata-only: stat never opens content, so a FIFO deliverable cannot block the parent for the provider's idle timeout; non-regular files (fifo/dir/symlink) degrade to UNVERIFIED. - containment canonicalized against the literal mount root: a final-component symlink or a swapped parent directory (root included) cannot redirect the check outside shared storage; unprovable layouts degrade to UNVERIFIED. * fix(harness): canonicalize probe containment against the canonical mount root (RFC #4651 PR4) Literal-root equality made every remote file leaf permanently UNVERIFIED on e2b and Tenki, which realize /mnt/user-data as a symlink to the home dir by default (e2b bootstrap 'sudo ln -sfn', Tenki best-effort symlink). Containment now compares the file's realpath against the mount root's realpath — exactly what the provider's own read path resolves, so probe and read-back stay consistent; final-component symlinks stay rejected by the non-dereferencing stat, and an intermediate dir-link escape under a sane root still lands ESCAPED. The inner script is a module constant and the suite now executes the composed probe for real against on-disk layouts (real dir, symlinked prefix, final symlink, fifo, missing, dir-link escape), which the canned-output stub could not see. * fix(harness): close bare-criterion negation and CDPATH summary channels (RFC #4651 PR4) - matching: a criterion with no positional selection target (bare pytest, make test) stands for the runner's default selection, so ANY negating option (--ignore/--deselect/...) makes the recorded run a different selection — unprovable. The overlap guard only sees consumed criterion tokens, which a bare criterion does not have; scoped criteria keep the unrelated-exclusion behavior. - attribution: cd is no longer blanket-silent — CDPATH makes cd print the resolved (subagent-chosen) destination and the pass shapes match as substrings, so one mkdir 'all tests passed' plus an export minted a pass for any quiet command. A cd argument or CDPATH= value (export or leading assignment) carrying any summary shape makes the segment non-silent; shape-free cd dir wrappers keep matching. - docs: _truncate_bash_output states the effective 32-char floor (the guarantee previously read as an unconditional max_chars bound). * fix(harness): close env-assignment and expansion channels in acceptance matching (RFC #4651 PR4) Self-audit in the shape of the last review rounds — channels the matcher classified as accounted-for that can change what runs, narrow the selection, or lend the summary text: - env assignments are no longer blanket-stripped: only an allowlist of inert display/CI knobs (CI, NO_COLOR, PY_COLORS, ...) may prefix a matched span, and a non-allowlisted assignment in any preceding segment (pure-assignment or export NAME=) is state pollution — PATH redirects the executable, LD_PRELOAD/PYTHONPATH/NODE_OPTIONS inject code, PYTEST_ADDOPTS/GOFLAGS/MAKEFILES inject selection-changing inputs, BASH_ENV runs arbitrary shell startup. All degrade to unprovable. - runtime expansions: any span token carrying /$( )/backticks, any negating-option value carrying an expansion or glob (unknown excluded set), and any extra executed token carrying glob metacharacters (crafted option-looking filenames narrow invisibly) are unprovable. Criterion-side globs stay self-consistent (literal match). - cd: an argument carrying a runtime expansion or glob is non-silent (unknown destination, unknown print); CDPATH= assignments are now handled as state pollution at the match layer, subsuming the value-shape special case. * fix(harness): persistent-shell evidence, exact env sets, option-arity scoping (RFC #4651 PR4) - tests_passed: on a persistent-shell provider (new Sandbox.persistent_shell_sessions capability, set by AioSandbox) every leaf degrades to UNVERIFIED — any earlier call in the shared session could have mutated the state the clean-looking run executed in, and only a fresh controlled session (RFC section 6 verifier) can prove otherwise. The flag is read from the provider registry without acquiring a sandbox. - env assignments: the allowlist is gone — no variable is provably inert across repositories (CI/DEBUG are routinely read by tests). The span's assignment prefix must equal the criterion's exactly (values included, order-insensitive); any assignment or export NAME= in a preceding segment is state pollution. - scoping: positional targets are now read by option arity, so a path embedded in an option (--basetemp=/tmp/p, --junitxml=/tmp/r.xml) never counts as a selection target and an extra positional after such a criterion narrows the default selection it denotes. * fix(harness): stamp shell provenance at harvest, close export/unset and arity gaps (RFC #4651 PR4) * fix(harness): split physical newlines as shell separators in acceptance matching (RFC #4651 PR4) * fix(harness): scope cd wrappers to thread data roots, pin accepted boundaries (RFC #4651 PR4) * fix(harness): preserve criterion connectors, prove file_written readable, fail-closed shell capability (RFC #4651 PR4) * fix(harness): compare only the connector prefix, tolerate trailing criterion semicolons (RFC #4651 PR4) * fix(harness): preserve continuation-line operators, keep ./-spelled executable identity (RFC #4651 PR4) * fix(harness): render criteria single-line so a multiline criterion cannot inject a forged checklist line (RFC #4651 PR4) * fix(harness): reject parent-traversal executable tokens in acceptance matching (RFC #4651 PR4) * fix(harness): reject parent-traversal negated values in acceptance matching (RFC #4651 PR4) |
||
|
早上肚子疼
|
3b601922ff
|
fix(buzz): move seen-event persistence off event loop (#5103)
* fix(buzz): move seen-event persistence off event loop * fix(buzz): address seen-event persistence review * fix(buzz): replace stale scheduled flush tasks * fix(buzz): harden final seen-event flush * fix(buzz): make seen-event shutdown retryable * fix(buzz): quiesce persistence after channel stop * fix(buzz): drain late events on repeated stop --------- Co-authored-by: zaoshangduziteng <309590849+zaoshangduziteng@users.noreply.github.com> |
||
|
PeaceMaker-best
|
137a3cb60d
|
fix(authz): recheck policy before sandbox reuse (#5006)
* fix(authz): recheck policy before sandbox reuse * fix(authz): avoid duplicate async sandbox checks * fix(authz): scope sandbox decision across middleware * fix(authz): construct async providers on the event loop * test(authz): avoid cold imports under Blockbuster --------- Co-authored-by: 嗜鵼 <hy2010hy2010@qq.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Zeren Wang
|
bb75f8d736
|
feat(sandbox): share sandbox identity derivation and acquire serialization (#4741) (#5089)
* feat(sandbox): share sandbox identity derivation and acquire serialization (#4741) Remote providers (AIO, E2B, BoxLite, Tenki, OpenSandbox) each inlined the same sha256(user:thread)[:16] sandbox-id expression and kept per-scope lock dicts that grew unboundedly until shutdown. This extracts both mechanisms into shared components without changing provider lifecycle, ids, capacity semantics, or public tool behavior: - sandbox/identity.py: keyword-only derive_sandbox_scope_token (byte-pinned compatibility contract) + is_sandbox_scope_token; per-provider golden vectors pin current behavior including BoxLite's raw-None quirk and each provider's private user_id resolution. - sandbox/acquire_serialization.py: AcquireSerializer — per-key lock table with holder/waiter refcount reclamation, bounded dedicated executor (async waits off both the event loop and the default executor), worker-owned cancellation cleanup (no event-loop callback dependency), idempotent close(). - Each provider adopts both components; AIO/E2B key by (user_id, thread_id) with acquire and (E2B) release serialized; BoxLite/Tenki/OpenSandbox key by derived sandbox id and offload the whole sync acquire to the serializer's executor so a cancelled awaiter cannot overlap a retried same-scope body (leaked-remote-VM regression caught in review). - thread_id=None acquires stay unserialized; provider shutdown()/reset() close the serializer; E2B capacity/ledger/reconciliation and AIO ownership/flock machinery untouched. - blocking-IO anchor proves contended OpenSandbox acquire_async stays off the event loop (teeth verified red/green); AGENTS.md documents the shared components. * refactor(sandbox): address review on acquire serialization (#5089) - Replace unreachable checkin branch with an assertion: run() returns False only after abandon(), which the except handler always re-raises; the old _checkin would have double-decremented the refcount. - Document the task.cancelling() == 0 assumption in hold_async. - Drop unused thread_id/user_id kwargs from BoxLite and Tenki _acquire_scope_locked (OpenSandbox still forwards them). * fix(sandbox): preserve request ContextVars in acquire executor bridge (#5089) loop.run_in_executor() does not copy contextvars, unlike the inherited SandboxProvider.acquire_async() which used asyncio.to_thread(). The BoxLite/OpenSandbox/Tenki acquire_async bridges introduced in this PR therefore dropped the request trace id (logged as trace_id=-). Add AcquireSerializer.run_on_executor(), which copies the calling context and runs the callable through ctx.run, and route all three providers through it. Add regression tests binding request_trace_context and verifying the worker thread observes it. |
||
|
ChaseMoon
|
cff8b74ec3
|
fix(harness): offload ACP workspace creation from event loop (#4965)
* fix(harness): offload ACP workspace creation from event loop * fix(harness): complete ACP event-loop offload |
||
|
ajayr
|
6cbf20fd39
|
feat(memory): add Honcho backend (user-model memory provider) (#4730)
* feat(memory): honcho backend config parsing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(memory): honcho v3 http client Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(memory): honcho memory manager (workspace-per-user, fail-closed identity, async offload) - HonchoMemoryManager implements the MemoryManager contract (add/get_context/ search/get_memory/shutdown_flush + aadd/aget_context/asearch offloaded via asyncio.to_thread), signatures verified against manager.py's tier-1/tier-2/ async abstracts. - Workspace resolution: workspace_overrides[user_id] else workspace_prefix + sanitize_id(user_id); missing/empty user_id fails closed (no-op write, empty read) rather than falling back to a shared workspace. User peer: user_peer_overrides[user_id] else sanitize_id(user_id). - get_context self-truncates to max_injection_chars and raises MemoryManagerError only under failure_policy.read=fail_closed; default is log-and-return "". - Restore backends/honcho/__init__.py to the noop direct-import convention (MANAGER_CLASS = HonchoMemoryManager) now that honcho_manager.py exists, replacing Task 10's temporary lazy __getattr__ scaffold. - Fix Task 10 deferred docstring minor: sanitize_id docstring now states the grammar allows up to 100 chars while this helper caps at 64. - 19 new tests appended to test_honcho_memory_backend.py (write/read/async/ lifecycle/factory-discovery); 27/27 pass. Verified end-to-end that manager.py's drop-in backend scanner resolves "honcho" with no core edits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(memory): collision-resistant identity derivation, exception containment, passive-writes flag Task review findings (2 Critical + 1 Important), all fixed in the same worktree: - CRITICAL (cross-user bleed): sanitize_id is lossy -- "user.name@example.com" and "user-name@example.com" both sanitized to the same string, merging two users' memory into one workspace/peer. Add _stable_id() (sanitize_id output + 8-hex-char SHA-256 suffix of the raw id) and use it on the default (non-override) path in _workspace/_user_peer; workspace_overrides / user_peer_overrides still match on the raw key, unchanged. The hash suffix also guarantees a non-empty result for a raw id that sanitizes to "" (e.g. "!!!"), so _user_peer can no longer return "". Documented in the manager's isolation docstring. - CRITICAL (exception containment): client.py's _post() called response.json() outside the try block, so a 200 with a non-JSON body raised a bare JSONDecodeError that would escape add() with no upstream handler. Wrap the parse and raise HonchoRequestError (mirrors Mem0Client._request). Broadened the manager's four boundary excepts from `except HonchoRequestError` to `except Exception` (mirrors openviking_manager.py's broad-guard precedent), with `except MemoryManagerError: raise` first so a contract error is never swallowed or double-wrapped. - IMPORTANT: added requires_passive_writes_in_tool_mode: ClassVar[bool] = True -- Honcho's only write path is passive add() (no fact CRUD hooks), so tool mode must keep MemoryMiddleware writes flowing to the deriver. Mirrors mem0_manager.py's identical flag/rationale. Minors addressed: get_memory(user_id=None) empty-shape-with-no-calls test; empty-string user_id tests for add()/get_context(); dedicated collision test proving two colliding raw ids resolve to different workspaces/peers. 10 new tests (37/37 total pass); RED verified by stashing only the implementation files (tests import the not-yet-existing _stable_id, so the whole module fails to collect) before restoring the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(memory): blocking-io anchor for honcho backend; docs + config example - Adds test_honcho_memory_backend.py in tests/blocking_io/ with fake-client blocking IO - Mirrors openviking anchor structure and conftest conventions - Updates backends/README.md with honcho row and config keys section - Updates config.example.yaml with honcho commented block - Updates backend/AGENTS.md with honcho memory backend bullet - Documents workspace resolution (prefix + collision-resistant sanitized id) - Documents tool mode passive write retention via MemoryMiddleware - Documents async entrypoint offloading via asyncio.to_thread - Documents fail_closed vs fail_open recall failure policy Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(memory): wire close() to shutdown hook; correct honcho README defaults and tool-mode note - HonchoMemoryManager.close() releases the HTTP client, mirroring mem0_manager.py's pattern and the base MemoryManager.close() shutdown hook. - README: fix workspace_prefix (deerflow-u-), message_char_limit (8000), max_injection_chars (6000), and base_url (default http://localhost:8000, not required) against backends/honcho/config.py; add missing timeout_seconds/connect_timeout_seconds rows; replace the "middleware mode only" claim with wording matching reality (tool mode supported, search implemented, passive writes retained via MemoryMiddleware). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(memory): honor failure_policy.read on all honcho recall paths; review nits Addresses PR #4730 review feedback: - search() and get_memory() now route through a _read_or_fallback policy gate (mem0's pattern), so failure_policy.read: fail_closed raises MemoryManagerError on every recall path as documented; get_context() uses the same helper, preventing future drift. - Session ids use the collision-resistant _stable_id derivation; bare sanitize_id would merge threads like "t.1"/"t-1" into one session. - HonchoClient accepts a transport kwarg (Mem0Client precedent) so tests inject httpx.MockTransport through the constructor. - Config: empty/null workspace/peer override values fail fast at parse time instead of silently falling through to the default derivation. - _UTC_NOW_FIELDS 1-tuple replaced by a plain _UTC_NOW_FORMAT constant. - README: user_peer_overrides row described the wrong target (it overrides the user's own peer, not assistant_peer); document the non-empty constraint on override values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(memory): qualify honcho isolation claim for shared workspace_overrides The module docstring claimed users cannot see each other's memory by construction, unconditionally. That holds for the default one-workspace-per-user derivation, but a workspace_overrides entry mapping several users to one workspace shares that workspace's search index: search() uses Honcho's workspace-scoped /search (no peer filter), while get_context()/get_memory() stay peer-scoped via working_representation. State the asymmetry in the docstring, the README Workspace Resolution section, and the workspace_overrides table row. Docs-only; no behavior change (review follow-up on #4730). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ryker_Feng
|
e401ae2d7b
|
feat(integrations): support switching Lark app credentials (#4703)
* feat(integrations): support switching Lark app credentials * fix(integrations): harden Lark app switching * refactor(integrations): simplify Lark switch flow * fix(integrations): reject superseded Lark flows * test(integrations): pass Lark flow generation * fix(integrations): preserve pending Lark flows --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hao Zhe
|
6556d09d7f
|
refactor(memory): use official OpenViking adapter (#4707)
* refactor(memory): use official OpenViking adapter * fix(memory): preserve OpenViking recall behavior * fix(memory): ignore ambient OpenViking headers |
||
|
Felix Wang
|
4795452102
|
fix(channels): offload outbound attachment file IO (#4633) | ||
|
DanielWalnut
|
459dd78707
|
perf(frontend): bound delivery, bundles, and long-running UI work (#4622)
* docs: design frontend performance remediation * docs: plan frontend performance remediation * test(frontend): add route asset performance budgets * perf(nginx): compress textual responses safely * perf(frontend): lazy load case study media * perf(frontend): bound static demo file tracing * perf(frontend): restore static locale boundaries * perf(frontend): defer closed workspace panels * perf(frontend): split editors and deduplicate highlighting * perf(frontend): index incremental message derivation * perf(frontend): stabilize paged history cache policy * perf(frontend): bound streaming markdown renders * perf(frontend): virtualize message history * perf(frontend): bound and virtualize chat lists * perf(frontend): suspend inactive decorative animation * perf(browser): stream latest frames as binary * perf(artifacts): stream bounded text previews * docs: finalize performance runtime boundaries * style(backend): apply test formatting * fix(frontend): keep translation functions client-side * perf(frontend): defer decorative animation bundles * test(frontend): lock optimized route budgets * fix: harden frontend performance boundaries * test(frontend): update i18n provider fixture * fix(frontend): preserve sidebar pagination position * style(backend): format artifact range test |
||
|
Ryker_Feng
|
5d8c4c2272
|
fix(feishu): keep file receive off the event loop (#4627) | ||
|
qin-chenghan
|
8234370a6a
|
feat(artifacts): inline editing for text artifacts in the panel (#4596)
* feat(artifacts): inline editing for text artifacts in the panel
Add a PUT /api/threads/{id}/artifacts/{path} endpoint that atomically
replaces an existing UTF-8 text file under /mnt/user-data/outputs after
verifying its SHA-256 revision. Active runs conflict (409); binary,
symlink, oversized, and non-output paths are rejected.
Frontend: edit/save/discard buttons, draft state with conflict detection,
CodeEditor onChange/onSave, loader SHA-256 from ETag, i18n, beforeunload guard.
Backend: PUT endpoint with thread reservation, atomic temp-file replacement,
sandbox sync for non-mounted providers, rollback on failure, ETag on GET.
Tests: 8 backend + 1 blocking-IO + 3 frontend test files.
* fix(artifacts): scope replacement permissions and release sandboxes
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Xinmin Zeng
|
f2e832330e
|
fix(sandbox): enforce disabled skills in filesystem views (#4178)
* fix(sandbox): project enabled skills into sandbox views * fix(skills): keep projection mutations consistent * fix(skills): fail closed on projection errors * fix(skills): isolate per-scope failures during boot projection rebuild rebuild_all_skill_projections() propagated any exception from the public rebuild or from a single user's rebuild straight out of the gateway lifespan startup, uncaught. A single broken user directory (bad permissions, corrupted _skill_states.json, unreadable content) would therefore abort gateway boot for every user, not just that one - _rebuild_*_locked already fails closed internally (clears the view and re-raises), so the boot loop only needed to stop treating that re-raise as fatal. Each scope's rebuild now fails closed independently and boot continues; a scope left empty by a boot failure self-heals on the next sandbox acquire via ensure_skill_projections(). Also patches deerflow.skills.projection.rebuild_all_skill_projections in the memory-flush lifespan test fixture, matching the two sibling fixtures in the same file — this call is now on the lifespan startup path and the fixture's minimal SimpleNamespace config predates it. * test(skills): update authz test for the projection-aware public toggle _persist_shared_skill_state (introduced earlier in this branch) reads the shared extensions_config.json fresh from disk under the projection lock instead of through the cached get_extensions_config() singleton - that's the whole point of the fix (stale worker caches must not clobber another worker's concurrent update). The name no longer exists on the skills router module, so the test's monkeypatch of it started raising AttributeError instead of exercising the endpoint. The mock storage in this test isn't a real LocalSkillStorage instance, so _persist_shared_skill_state's projection-mutation branch is already skipped (nullcontext) and it falls back to a fresh ExtensionsConfig() for the nonexistent tmp config_path - no replacement monkeypatch needed. * fix(sandbox): make skill projection ensure best-effort in acquire acquire() called _ensure_skills_projection() directly, outside any try/except, in both LocalSandboxProvider and AioSandboxProvider. Every other skill-mount setup path in these providers has always caught exceptions and logged a warning rather than failing sandbox acquire outright (e.g. when config.yaml can't be resolved) - these two new call sites broke that contract, so any projection failure (including simply not having a config.yaml, as in CI's test environment) now failed acquire() itself instead of just leaving skill mounts off. _ensure_skills_projection now catches its own exceptions and returns None; both providers' callers already tolerate that (a None projection skips the skill-specific mounts, matching the existing degrade path) after making _append_public_skill_mapping and the custom/legacy mount block in LocalSandboxProvider explicitly None-safe. Caught by running the full suite with config.yaml removed, matching CI's environment - not caught locally because a real config.yaml was present, masking the failure. * fix(sandbox): make E2B skill projection mounts best-effort _skill_projection_mounts called ensure_skill_projections with no guard, unlike Local/AIO's _ensure_skills_projection. A raise propagated out of _apply_mounts before the configured-mounts loop ran, so a skills projection failure dropped the operator's own configured mounts too - only caught by create()'s outer warning, with nothing applied at all. Swallow here and return an empty mount list on failure, matching the Local/AIO pattern: still fail-closed for skills, but no longer widens the blast radius to unrelated configured mounts. Review feedback from PR #4178. * docs(skills): document projection trade-offs flagged in review - _update_tree_digest: note the metadata-only (not content) hashing trade-off and why runtime writes through this codebase are still covered regardless (rebuild-under-lock + rename always changes inode). - LocalSandboxProvider.acquire: note the acquire-time self-heal cost (cheap on a fresh manifest, ~400ms rebuild under lock on stale/drift). - skill_projection_mutation: drop the no-op except-Exception-then-raise; a raise from the mutation already propagates past the yield with the view left cleared, no explicit re-raise needed. - provisioner README: spell out that hostPath skills volumes require the gateway and K8s node to share DEER_FLOW_HOST_BASE_DIR (single-node or shared storage), and that the custom/legacy volumes' hostPath type Directory (not DirectoryOrCreate) makes a violation of that assumption a visible Pod-creation failure instead of a silent empty mount. Review feedback from PR #4178. * fix(skills): lazily repair user projections * fix(skills): close projection review gaps * fix(skills): refresh user projection enable state * fix(skills): close projection review follow-ups * fix(skills): preserve state across projection writes --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huixin615
|
133a82c6c2
|
fix: isolate MCP server toggles from invalid peer configs (#4577)
* fix: isolate MCP server toggle updates * fix: write extensions config atomically * fix: normalize MCP transport aliases |
||
|
ly-wang19
|
4582a41347
|
fix(skills): offload blocking filesystem IO in update_skill and serialize writes (#3565)
* fix(skills): offload update_skill's blocking IO and share the config write lock Re-applied on top of main rather than merged: update_skill has since gained admin gating, user-scoped storage and a PUBLIC vs CUSTOM/LEGACY split, so the offload is applied per path. - PUBLIC: the extensions_config.json read-modify-write (path resolve, snapshot, merge, write, reload) moves to a worker thread via asyncio.to_thread. The payload is built from a snapshot so the cached singleton is never mutated in place while the write is still in flight. - CUSTOM/LEGACY: set_skill_enabled_state is offloaded; the non-user-scoped fallback takes the same shared-file RMW path as PUBLIC. - Both load_skills calls (and storage construction) are offloaded. The RMW lock now lives next to reload_extensions_config as get_extensions_config_write_lock() and is acquired by both the skills router and the MCP router, which performs the same read-modify-write on the same file. Previously each router held its own module-local lock, so once both sides offloaded, a PUT /api/mcp/config could run inside a skill toggle's read->write window and the later write would silently drop the other's change. The lock is keyed by the running event loop rather than being a module-level singleton: asyncio primitives bind to the first loop that awaits them, which makes a plain module-level lock unusable in a process that runs more than one loop. Adds a cross-router regression anchor asserting a skill toggle and an MCP update never overlap inside the RMW (max in-flight 1); it observes 2 when the routers use separate locks. * fix(config): own the extensions_config RMW lock from the worker thread An asyncio.Lock held around `await asyncio.to_thread(...)` protects only the awaiting task. If that task is cancelled the context manager releases the lock immediately while Python keeps running the worker thread, so a second skills or MCP writer could acquire it and operate on extensions_config.json concurrently with the first worker — reopening the lost-update window this was meant to close. The per-event-loop keying had a second hole: writers on different loops got different locks and so did not exclude each other at all. Replace it with a process-wide threading.Lock acquired *inside* the worker that performs the RMW (`_write_extensions_skill_state` and `_apply_mcp_config_update`), so ownership belongs to the thread doing the writing and is held until the write and reload actually finish, regardless of what happens to the caller. A threading.Lock also has no event-loop affinity. Adds a cancellation regression: the skills worker is paused inside the lock, its route task is cancelled, and the MCP writer is started — the MCP RMW must not enter until the skills worker is released. Against the previous asyncio-lock design this test fails with ['skills-enter', 'mcp-enter']. The two existing serialization tests instrumented by replacing the worker functions, which now bypasses the lock under test; they instead patch inside the real workers (each module's reload_extensions_config, the last step under the lock) so the production lock is exercised. --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
RongfuShuiping
|
2aaf74b0f8
|
feat(memory): add OpenViking HTTP backend (#4509)
* feat(memory): add OpenViking HTTP backend * fix(memory): harden OpenViking lifecycle --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
阿泽
|
ea74367502
|
fix(runtime): honor LangGraph Server identity for user-scoped data (#4538)
* fix(runtime): honor LangGraph Server identity for user-scoped data * fix(runtime): scope custom agent SOUL by resolved user |
||
|
Fgoll
|
62b73fd2ea
|
feat(dingtalk): support inbound file and image attachments (#4423)
* feat(dingtalk): support inbound file and image attachments DingTalk previously dropped picture and file (document) messages because `_on_chatbot_message` ignored any message with empty text, so users could not send files to the agent. This adds inbound attachment support, mirroring `FeishuChannel`: - `_extract_files` parses `picture`/`richText` image downloadCodes and `file` (document) descriptors. `dingtalk_stream.ChatbotMessage.from_dict` does not parse `file` messages, so `_DingTalkMessageHandler.process` stashes the raw callback payload on the message (`_df_raw_data`) for the document descriptor. - `receive_file` downloads each attachment by `downloadCode` via the robot `messageFiles/download` OpenAPI, persists it into the thread uploads bucket, syncs it into a non-local sandbox, and prepends the sandbox virtual path to the message text so the agent can read the file by path. - Filenames go through the shared `uploads.normalize_filename` helper, which strips directory components and rejects traversal patterns. Outbound `send_file` already existed; this completes DingTalk file parity with Feishu on the inbound side. Adds 21 tests covering extraction, download-by-code, persistence/sandbox sync, filename sanitization, and the handler raw-data stash. * fix(dingtalk): address inbound-file review feedback Follow-up to the review on #4423: - Make the fallback filename safe by construction. `download_code` is attacker-controllable webhook data and was embedded into `fallback_name` unsanitized; it only avoided escaping the uploads directory because the resulting write failed with OSError. It is now restricted to `[A-Za-z0-9_-]` before use. Covered by a test that reproduces the old behaviour (`uploads/dingtalk_../../evil.png`) and by a test that actually exercises the previously untested `except ValueError` branch (`".."`, whose basename — unlike `../../etc/passwd` — does raise). - Log the swallowed `get_image_list()` failure instead of silently returning no images, so an SDK parse failure is distinguishable from a richText message that genuinely has no inline images. - Surface failed downloads to the agent as a short `[failed to load ...]` marker rather than silently omitting the attachment, so a user whose file did not load does not simply appear to be ignored. Keeps the cleaner text shape while restoring the signal Feishu provides. Tests: 119 passed (was 115). * fix(dingtalk): claim unique upload names and refuse symlinked destinations Round 2 review follow-up on #4423. Both findings reproduce as failing tests against the previous head. - Inbound attachments no longer overwrite each other. Generated names repeat across messages (every picture message yields "image.png", richText yields "image_0.png"), so a later attachment silently replaced an earlier one whose virtual path had already been prepended to the message text — the agent could read bytes that were not the ones its prompt referenced. The destination name is now claimed with the shared `claim_unique_filename` against the live directory contents, which also covers a real filename sent twice (`quote.xlsx`), a case Feishu's inline naming does not handle either. The claim and the write happen under one lock so two attachments cannot resolve to the same free name. - Writes go through the shared `write_upload_file_no_symlink` instead of `Path.write_bytes`. Uploads dirs may be mounted into local sandboxes, so a sandbox process could leave a symlink at a future upload name and redirect a gateway-privileged write outside the bucket; the regression test shows the old code creating the out-of-bucket target. Tests: 123 passed (was 119). * fix(dingtalk): harden the inbound download path (self-audit) Proactive hardening pass over the new inbound path; each fix reproduces as a failing test against the previous head. - Contain token failures. `_get_access_token()` sat outside the try in `_download_by_code`, and the manager awaits `receive_file` without one — a DingTalk auth hiccup during a file message aborted the whole chat turn with no reply. Token acquisition moves inside the try, and `receive_file` gains per-attachment isolation so no unforeseen error can escape past the marker. - Cap inbound size. The download buffered arbitrary bytes in memory (`response.content`) with no limit, while outbound uploads already enforce one. The body is now streamed and dropped once it exceeds `_MAX_INBOUND_FILE_SIZE_BYTES` (50 MB), surfacing as a failed-load marker. - Sanitize the failure marker. It embedded the raw webhook `fileName`; a newline could forge a standalone `/mnt/user-data/uploads/...` line inside msg.text and an over-long name bloated it. Markers now collapse whitespace and cap at 80 chars. - Keep blocking IO off the event loop. `ensure_thread_dirs`, the uploads-dir resolve, sync `SandboxProvider.acquire`, and `sandbox.update_file` all ran on the loop; directory prep now lives inside the same `asyncio.to_thread` as the claim+write, and sandbox sync uses `acquire_async` + an offloaded `update_file`. Locked by a strict Blockbuster anchor (tests/blocking_io/test_dingtalk_receive_file.py), verified to fail with `BlockingError: Blocking call to os.mkdir` when the offload is reverted. Tests: 127 + 1 blocking-io anchor (was 123); tests/blocking_io/ suite 55 passed. * fix(dingtalk): surface missing-sandbox sync as a failed load Round 3 follow-up on #4423: - When a non-local sandbox acquire succeeds but the provider cannot resolve the instance, _receive_single_file returned the virtual path anyway — a path the agent's sandbox cannot read. Mirror Feishu: log and return "", so the [failed to load ...] marker fires instead. Red-first test: test_missing_sandbox_after_acquire_yields_marker. - Drop the dead GetResponse / FakeClient.get scaffolding left in test_oversized_download_is_dropped from its red-first iteration. Tests: 128 + 1 blocking-io anchor (was 127 + 1). * fix(dingtalk): treat non-local sandbox sync failure as a failed load Round 4 follow-up on #4423. The sync except-branch logged and still returned the virtual path when acquire or update_file raised on a non-local sandbox — the same handing-the-agent-an-unreadable-path failure mode the sandbox-is-None branch was just fixed for, and exactly the leg the suite did not exercise. Feishu's except-branch returns its failure marker; DingTalk now does the equivalent (return "" so the failed-load marker fires). Red-first test: test_update_file_failure_yields_marker. Tests: 129 + 1 blocking-io anchor (was 128 + 1). --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Vanzeren
|
1c7531242c
|
feat(runtime): record terminal artifact delivery receipts (slice 1 of #4272) (#4365)
* feat(runtime): record terminal artifact delivery receipts (#4272) * fix(runtime): persist delivery receipts across recovery * test(runtime): cover delivery receipt invariants * fix(runtime): preserve terminal status on receipt outages |
||
|
Daoyuan Li
|
5d073991c2
|
fix(sandbox): widen boxlite/aio_sandbox tenant hash and verify identity on reclaim (#4171)
* fix(sandbox): prevent truncated tenant ID reuse * fix(sandbox): handle late same-tenant box registration |
||
|
Ryker_Feng
|
7aa314b4c1
|
feat: add Lark CLI integration (#3971)
* feat: add lark cli integration * fix: polish lark integration actions * feat: support lark incremental permissions * fix: detect lark authorization completion * fix: harden lark integration install * feat: expand lark auth scopes and reuse host auth in sandbox Default lark auth to least-privilege (recommend=false, base sign-in only) and expose the full set of lark-cli --domain business domains as native --domain grants instead of a 4-domain read-only mapping. Resolve the skill pack from the latest larksuite/cli GitHub release at install time with content-hash integrity, and surface version/runtime drift in status. Share the per-user lark-cli config/data profile between the Gateway Settings auth flow and agent conversations by mounting the integration dirs into the AIO sandbox and injecting the matching env for lark-cli commands, with an allowlisted extra_mounts path in the provisioner/K8s backend and traversal guards on integration paths. * style: fix lint issues from ruff and prettier Sort imports in the provisioner PVC test and re-wrap two long i18n description strings to satisfy backend ruff and frontend prettier CI. * fix(lark): address managed integration review feedback * fix(frontend): stabilize integrations settings e2e * test(sandbox): isolate remote backend legacy visibility check * test: fix backend unit failures after merge * Harden Lark integration review fixes * Format Lark integration E2E test * fix(lark): harden sandbox credential exposure and status disclosure Address willem_bd's security review on PR #3971: - Mount the per-user lark-cli config dir (long-lived appSecret) read-only into the AIO sandbox; only the refreshable-token data dir stays writable. - Redact host filesystem paths (install_path, cli.path) from GET /lark/status and the config/auth complete responses for non-admin callers, fail-closed on any auth error. - Document the npm postinstall trade-off (--ignore-scripts is not viable because @larksuite/cli fetches its platform binary in postinstall). - Document the sandbox credential trust boundary in AGENTS.md and README, pointing at the sidecar-broker follow-up (#4338). --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
ly-wang19
|
25d9ac0a43
|
fix(skills): offload blocking filesystem IO in get_custom_skill_history (#3563)
* fix(skills): offload blocking filesystem IO in get_custom_skill_history
The GET /api/skills/custom/{name}/history handler ran its storage probes and the
per-skill .history read directly on the asyncio event loop:
get_or_new_skill_storage(), custom_skill_exists(), get_skill_history_file().exists()
and read_history() are all blocking filesystem IO. make detect-blocking-io flagged
the existence probe (routers/skills.py:224) as DIRECT_ASYNC.
Move the whole read into a nested sync function run via asyncio.to_thread; a None
return signals 404 (distinct from an empty history list). Behavior is unchanged.
Per the blocking-io-guard SOP:
- Candidate: get_custom_skill_history (FILE_METADATA, DIRECT_ASYNC) -> FIX+ANCHOR.
- Re-scan: the finding no longer appears for this handler.
- Anchor: tests/blocking_io/test_skills_router.py drives the real handler against a
real on-disk skill + history; teeth verified red (pre-fix) -> green (post-fix)
under make test-blocking-io.
Scoped to this self-contained read handler. rollback_custom_skill and update_skill
also touch blocking IO but interleave it with awaits (security scan / cache refresh)
and do a read-modify-write, so offloading them needs the asyncio.Lock serialization
treatment (cf. #3552) and is left as a separate fix unit.
* test: trim dead skills history setup
* fix(skills): use the user-scoped storage accessor in the offloaded history read
The merge with main left the offloaded reader calling get_or_new_skill_storage,
which is not defined in this module (ruff F821), so lint failed and the handler
would raise NameError at runtime. Use _get_user_skill_storage(config) — the same
accessor every other handler in this router uses.
Also update the regression test for the current route signature: the handler is
now admin-only and takes a Request, so the test supplies request.state.user
(mirroring tests/blocking_io/test_channel_runtime_config_store.py) and seeds the
history through the same user-scoped accessor.
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Aari
|
0d4d0cb17d
|
feat(agents): database-backed storage for custom agent definitions (#4359)
* feat(agents): database-backed storage for custom agent definitions Add an agent_storage.backend switch (default file, behaviour-unchanged) with a db backend that stores each custom agent as a row in the shared SQL persistence layer, so a multi-instance deployment sees the same agents on every node (#4331, #4357). Introduces an AgentStore interface routing all read/write surfaces, an agents table + migration 0006, startup validation, and a file->db importer. Follows the thread_meta store / run_events backend-switch / 0003_scheduled_tasks migration patterns; no new dependency. * fix(agents): make db storage path production-ready (review round 1) Addresses review feedback on the db/sync agent-storage path: - sql.py: mirror the async engine's per-connection SQLite PRAGMAs on the sync engine (busy_timeout=30000, synchronous=NORMAL, foreign_keys=ON, WAL) so both engines behave identically against the shared DB; guard the engine cache with a lock (double-checked) so concurrent first-touch cannot build duplicate engines or register the connect listener twice. - routers/agents.py + routers/assistants_compat.py: offload the sync-store reads that ran on the event loop (list/get/check, update's pre-read + legacy guard + refresh, and assistants_compat's four list routes) via asyncio.to_thread — on db+postgres each was a network round trip stalling the loop. Writes were already offloaded. - file.py: translate the create() mkdir(exist_ok=False) race FileExistsError into AgentExistsError (router 409, matching SqlAgentStore's IntegrityError path); correct the _write docstring — per-file atomic replace, two commits sequential not transactional. Tests: sync-engine PRAGMA + engine-cache reuse assertions; file create-race -> AgentExistsError; strict Blockbuster anchor over the read endpoints so a regression back onto the loop fails CI. * fix(agents): address round-2 review on the db store path - update_agent tool: align the docstring/inline comment with FileAgentStore._write. Cross-field write atomicity is db-only; the file backend commits config then soul via two sequential os.replace (a crash between them can leave a fresh config.yaml beside a stale SOUL.md). The dropped partial-write *reporting* is an intentional tradeoff — the stage-then-replace safety is preserved (test_update_agent_soul_failure_does_not_replace_config still holds). - SqlAgentStore.update(): true upsert. Catch IntegrityError on the insert-on-missing branch, re-fetch and apply, so two concurrent first-time writes (e.g. two setup_agent handshakes) converge instead of surfacing a raw UNIQUE(user_id, name) violation as a 500. Symmetric with create(). - get_agent_store(): document the graph-subprocess config-resolution invariant (the except->file fallback is a genuine no-config path, not a mask for a misconfigured graph process) and pin it with two tests driving the real get_app_config() file resolution: db resolves from an on-disk config.yaml, file fallback when config is unresolvable. * test(agents): cover SqlAgentStore.update() write-race upsert recovery Mandatory-TDD test for the round-2 fix in 0680340a: two concurrent first-time update()s where the loser's insert hits UNIQUE(user_id, name). Deterministically forces the IntegrityError recovery path by making the first _row probe miss the committed winner, and asserts last-writer-wins instead of a surfaced 500. |
||
|
Aari
|
5eb59cb130
|
fix(sandbox): stop multi-worker orphan reconcile from killing peer sandboxes (#4221)
* fix(sandbox): stop multi-worker orphan reconcile from killing peer sandboxes Docker sandboxes are shared across gateway workers, but each worker kept its own in-memory warm pool. Startup reconciliation adopted every running container, so a peer idle reaper could destroy sandboxes another worker still owned and tool calls hit 502 / Connection refused. Add file-based ownership leases under sandbox-leases/, only adopt true orphans, refuse idle/replica/shutdown destroy while a foreign lease is live, and renew the lease on create/get/release/reclaim. Fixes #4206 * fix(sandbox): close lease fail-open, hot-path IO, and check→destroy race Address review of the multi-worker orphan lease (#4206): - read_lease returns None only for a genuinely-absent lease and raises (CorruptLeaseError/OSError) when a lease is unreadable or corrupt, so the ownership check fails closed instead of mistaking an unprovable peer lease for a free container. clear_lease still removes a stuck/corrupt file. - get() no longer renews the lease (blocking mkdir/fsync/os.replace on the event loop path used by ensure_sandbox_initialized_async); active leases are renewed off the event loop from the idle checker (_renew_active_leases). - The ownership check and container stop run under a per-sandbox flock guard (lease_ownership_guard); every lease write takes the same guard so a peer's touch cannot interleave with a destroy. Same-host multi-worker scope, not a multi-pod distributed lock. Also fixes the ruff format lint on the branch. Adds regression tests: corrupt and unreadable lease fail closed, a tests/blocking_io anchor keeping get() non-blocking on the event loop, and a peer-touch/destroy interleave test. * fix(sandbox): share container ownership across gateway instances Rework of the #4206 fix per review: ownership state is shared through a third-party service instead of being maintained per gateway instance, following the stream_bridge precedent (sandbox.ownership.type: memory | redis). The file lease and its same-host flock guard are deleted, not ported — they only covered workers on one host, while the deployment that hits #4206 is a load-balanced multi-instance gateway. A lease answers "who reaps this container", not "who may use it". Containers are deterministic per (user, thread), so consecutive turns legitimately land on different instances: take() transfers ownership on acquire, while claim() gates every adopt/reap path. Leases carry a state — own: or del: — so a takeover is refused against a teardown in progress. Without it an unconditional take() would overwrite a destroyer's claim and the peer's container stop would land on a sandbox the new owner had already handed to an agent. renew() distinguishes a lapsed lease from one a peer took; only the latter drops the sandbox. Collapsing them meant a Redis restart evicted every in-flight sandbox on every instance at once. Renewal runs on its own thread with a TTL derived from its interval, never from idle_timeout: renewal used to ride the idle checker, which does not start at idle_timeout: 0, so leases silently lapsed on a supported config. Ownership establishment is fail-closed: a sandbox whose ownership cannot be published is never handed out, and a just-created container is destroyed rather than leaked as an adoptable orphan. Every destroy path claims before untracking. The memory store is single-instance only and says so; the resolver reads app_config.stream_bridge and the env var in the bridge's own order, so deployments already using Redis get a redis ownership store without extra config. * fix(sandbox): wait out a recovery grace before adopting a keyless container An absent ownership lease meant two opposite things on two paths. Renewal reads it as LAPSED and re-establishes it: nobody took the lease, so the container is still ours. Reconciliation read the same absent key as "orphan" and adopted on sight. After the store loses its keys (a Redis restart without persistence, or eviction under maxmemory) every owner is alive and merely pre-renewal-tick. Whichever instance reconciled first therefore adopted every live container; each real owner's next renewal reported LOST and dropped a sandbox it was serving mid-turn, leaving it for the adopter to idle-destroy — #4206 through the back door, in the very case the LAPSED handling was added to make safe. Not limited to startup: an already-running instance hits the same window from the idle checker's periodic reconcile. _adoptable_after_grace requires an untracked container to be seen unowned across a full lease TTL before it can be adopted. That rebuilds the delay the state loss erased: a live owner republishes within one renewal interval, shorter than the TTL by construction, while a crashed owner never does, so its containers are still adopted one grace later rather than leaking. A republished lease resets the grace; a pausing-only timer would still expire over a live owner's lease. The peek is read-only — the atomic claim still gates adoption. The grace is skipped when the store cannot coordinate across processes: no peer can hold a lease such a store would show us, so single-instance deployments keep instant orphan cleanup, and a grace could not help a multi-worker gateway on memory anyway. * fix(sandbox): hold the teardown lease for as long as the container stop runs claim(..., for_destroy=True) wrote the del: marker with the ordinary lease TTL and nothing refreshed it. renew() extends only own: and deliberately reports a teardown as LOST, and the destroy paths drop the sandbox from the maps the renewal loop iterates — so a container stop that outlived the TTL let the marker lapse, a peer's take() succeeded against the still-running container, and the stop then landed on the turn that had just been handed it. That is the exact window the del: state exists to close, reopened by its own expiry. The two lease states alone never made the per-sandbox flock redundant, as I claimed when deleting it: a held lock cannot expire, a lease can. The exclusion has to be held deliberately rather than assumed to outlast the work it guards. _held_teardown_lease wraps both _backend.destroy() call sites and re-claims the marker every renewal_interval_seconds until the stop returns. No store change is needed: claim(for_destroy=True) already refreshes an existing del: marker on both backends. Reachable without an abnormal backend. The schema bounds only renewal_interval_seconds (> 0) and ttl_multiplier (>= 2), so a legal config puts the TTL below a normal container stop; and LocalContainerBackend._stop_container passes no timeout to subprocess.run, so a wedged daemon blocks unbounded even at the default 120s TTL. The TTL stays finite on purpose: the heartbeat dies with the process, so a destroyer that crashes mid-stop still releases the container one TTL later instead of marking it undestroyable forever. * fix(sandbox): hold the teardown lease on every del: stop, and pin the claims that had no test 90936b49 said `_held_teardown_lease` wrapped "both" `_backend.destroy()` call sites. There are three. `_drop_unhealthy_sandbox` marks `del:` and then blocks on the same unbounded stop, and it untracks *before* claiming, so `_renew_owned_leases` cannot see the id either — nothing refreshed the marker. Reproduced against a real redis: the peer's `take()` succeeds 1.0s into a 2.5s stop. Same window, third path. That miss came from the habit the rest of this commit addresses: a property asserted in prose, with no test that could falsify it. Auditing every load-bearing claim in this feature — AGENTS.md, the store docstrings, the provider's design comments — against the test that would go red turned up several more, each verified by mutating the code and watching the suite stay green. Tests that could not fail: - `test_reconcile_fails_closed_when_ownership_unknown` reached the grace gate, not the claim. A bare MagicMock answers `owner()` with a truthy mock, so the container read as peer-owned and deferred; `claim()` was never called. It stayed green with `_claim_ownership` failing open. Adding the grace ahead of the claim is what hollowed it out — inserting a gate can silently disarm the tests for the gate behind it. - `test_adoption_grace_restarts_when_a_live_owner_republishes` never distinguished reset from pause. Those diverge only on a *second* lapse, which it never drove, so it passed with the reset deleted. Claims with no test at all, each now pinned (mutation → red, per test): - `destroy()`, `_evict_oldest_warm`, `_reclaim_warm_pool_sandbox`, `_register_created_sandbox` and `shutdown()`'s warm loop were each the one untested sibling of an "every path does X" enumeration. `shutdown()` was never driven with a non-empty warm pool, so a loop bypassing the ownership claim — stopping a live peer's container on our exit — went unnoticed. - Renewal's unknown-is-not-lost rule, the single deliberate exception to fail-closed. Inverting it drops every active and warm sandbox on every instance the moment the store blinks. - Both hops of the stream-bridge redis inference. Deleting either left the suite green while every config.yaml-native multi-instance deployment silently fell back to memory — #4206 reopened on exactly the deployments the inference exists for. Claims narrowed instead, because they promised more than the code delivers: - "run against both backends ... cannot drift" — CI provisions no redis, so the merge gate runs the memory tier only and the Lua never executes there. - "Every destroy path claims before untracking" — `_drop_unhealthy_sandbox` untracks first, deliberately, under its `expected_info` TOCTOU guard. - "Atomic: concurrent claims from different instances cannot both succeed" — true via Lua on redis, vacuous on the single-instance memory store, and pinned by neither, since the contract suite drives sequential calls. A concurrency test against the memory store would make the claim look covered while the mechanism that carries it still never runs in CI. * fix(sandbox): release the teardown marker when a destroy() stop fails The three `del:`-marked stop paths disagreed on failure. `_destroy_warm_entry` releases on both outcomes and says why: the stop failed, so the container is probably still up, and a marker left behind refuses its own thread's `take()` until the TTL lapses. `_drop_unhealthy_sandbox` does the same. `destroy()` had no such guard — a raising backend propagated straight past `_release_ownership`, and the thread could not re-acquire for a full TTL. Fails safe rather than fatal: a stuck marker stops peers from touching the container, it is not the cross-instance kill. But the paths must agree, and this one is the odd one out. Release, then re-raise. Swallowing would be the easier symmetry with `_destroy_warm_entry`'s `return False`, but `destroy()` has no failure return and `shutdown()` logs per sandbox off the exception, so swallowing would silently narrow what callers can see. Found by comparing the three paths after @fancyboi999 asked for release to be handled "consistently with the other destroy paths" on the unhealthy path — which 0d2377b2 already does. This is the sibling that wasn't. * fix(deploy): bump chart config_version to 27 for sandbox.ownership config.example.yaml went to 27 with the new sandbox.ownership section, but the chart embeds its own copy and stayed at 26, so validate-chart failed. A bare bump: the chart already sets stream_bridge.type=redis, which is what resolve_ownership_config infers a redis ownership store from, so no field change is needed. * fix(sandbox): release the teardown lease from its heartbeat, not the caller `_held_teardown_lease` joined its heartbeat only briefly and the caller cleared the `del:` marker right after the stop. A refresh `claim` still in flight (`RedisOwnershipStore` had no socket timeout, so a round trip could block) could land *after* that release and rewrite `del:` on a container whose stop had already completed — refusing a fresh `take()` (or rolling back a fresh create) until the TTL. Move the release into the heartbeat's own `finally`, after its loop stops, so no refresh can run after it. The three destroy paths no longer release after the `with` (`destroy()`'s no-container branch still does, since no lease was held there). Bound every store round trip with a socket timeout so the in-flight refresh — and thus the deferred release — stays finite, and broaden the heartbeat's `except` so an unexpected error cannot strand the marker during a long stop. Also fold in the review follow-ups: stop re-resolving an already-resolved ownership config in the factory, document the Redis-outage-vs-TTL boundary in config.example.yaml, and add a tests/blocking_io anchor pinning that `release()`'s store round trip stays off the event loop. * fix(sandbox): refuse a non-destroy claim that would unwind our own teardown `claim(for_destroy=False)` against our own `del:` lease fell through and overwrote it with `own:`, cancelling a teardown that was already in flight. The container stop cannot be recalled, so downgrading the marker would let a `take()` hand out a container that is about to die -- #4206, self-inflicted. No caller does this today: the two non-destroy callers run against an absent key (the LAPSED re-claim) or an unowned one (post-grace reconcile). The contract has to forbid it rather than rely on that staying true. Fixed in both backends. The redis rule lives in Lua and the memory rule in Python, so fixing one only would let them drift silently -- and the shared contract suite is what is supposed to catch that drift, so it now covers this. Also adds a contention test for `claim`. The suite drove sequential calls only, so it pinned the exclusion predicate but not the atomicity that predicate depends on; eight instances now race for one container and exactly one must win. * fix(sandbox): bound the container stop so it cannot outlive its teardown lease `_stop_container` passed no `timeout` to `subprocess.run`, so a wedged container runtime blocks it forever. The `del:` marker is what keeps a peer from re-acquiring the container while the stop runs, but a marker is a lease and a lease can lapse: a store outage longer than the TTL frees it, a peer's `take()` succeeds against the still-running container, and the stop then lands on the turn that was just handed it -- the exact #4206 failure. The teardown heartbeat already covers the case where the store stays reachable. This bounds the worst case independently of the ownership layer, which is the point: it holds even when the ownership layer is the thing that failed. A timeout is not swallowed like a `CalledProcessError`. That error means the runtime answered "I could not stop it"; a timeout means we do not know, and the container is probably still running -- returning normally would let `_destroy_warm_entry` report a clean stop and drop the warm entry, leaking a running container nothing tracks. * fix(sandbox): exclude this instance's own reapers from its acquire path An ownership lease excludes peers and nothing else. `claim()` and `take()` both succeed against our own `own:` lease by design -- that is what lets a destroy path claim what it already owns -- so `del:` says nothing to this process's other threads. Meanwhile every reaper decides outside `_lock`, because a store round trip must not be held under the lock that guards every acquire. So each reaper acts on a decision its own acquire path may already have invalidated, and the store cannot see the difference. Six paths end in an irreversible act (a container stop, or closing a host-side client) on a decision made outside the lock. All six reproduce: _evict_oldest_warm re-checks warm membership, then releases the lock _reap_expired_warm no re-check at all _cleanup_idle_sandboxes re-verifies idle, then releases the lock _renew_owned_leases acts on a stale renew() -> LOST release() same staleness on its own refresh _drop_unhealthy_sandbox untracks before claiming, opening discovery Both warm reapers are a regression from the deferred pop this branch introduced: `WarmPoolLifecycleMixin` popped under the lock, so a reclaim's membership check failed and the race could not occur. Deferring the pop is still right (popping first loses the container on a refused claim), so the exclusion has to be made explicit instead. The idle path is pre-existing in shape, but this branch widened it from a few instructions to a network round trip by claiming ownership before untracking. Two guards, because the two directions want opposite answers: Reaping -- nothing may promote it. The reaper reserves the id, and every promote path refuses a reserved id exactly as it refuses a peer's `del:` (drop and cold-start). The "is this still reapable?" test travels with the reservation as a predicate and runs in the same critical section, because checking first and reserving second is the window, not a narrower version of it. Forgetting -- the peer legitimately wins, so the promote is what to detect. `_publish_ownership` bumps a per-id acquire epoch; the callers that decide from a store round trip snapshot it first, and the pop is skipped if it moved. Object identity cannot substitute: the reuse path re-publishes ownership while handing out the same tracked `AioSandbox`, so an identity check sees nothing and the pop closes a client mid-turn. `still_reapable` is required rather than defaulting to unconditional -- the safe default is the one that makes a new call site think about it. That diverges from the mixin hook, which is safe because this provider overrides both mixin callers, and loud rather than silent if those are ever dropped. Also closes a client leak on the discover path: "nothing to roll back" was true of the container but not of the HTTP client constructed before the publish, which the sibling create path already closes. The shared-store test view rebound `owner_id` outside the store's lock, so a concurrent claim could execute under the wrong id and read its own lease as a peer's. Serialized, so the heartbeat-hold tests stop flaking. * fix(sandbox): mark acquire intent before the ownership round trip A guard must become visible no later than the transition it guards. The acquire epoch cannot manage that for `take()`: the takeover is durable before `take()` returns -- redis has committed the SET while the reply is still in flight -- and the epoch can only be written afterwards. In that interval the store already says the container is ours while the epoch still reads as it did when a renewal decided `LOST`, so the stale forget walks through, drops the maps and closes the client the acquire is about to hand back. Acquire then returns an id the provider no longer tracks and `get()` answers `None` for the rest of the turn. `_publish_ownership` now publishes an intent mark under `_lock` before the round trip; the epoch keeps covering the other half, "an acquire completed since you decided". `_forget_lost_sandbox` honours the intent mark unconditionally rather than only when an epoch is supplied -- today's epoch-less callers cannot reach the window, but "no epoch" reading as "no guard" is how the next caller of a dangerous primitive gets written. The same invariant had four more instances, all reproduced: reuse returns a decision the forget already invalidated -- before the mark is set a `LOST` is both current and correct, so the forget legitimately runs and the entry reuse decided to hand out is gone. Re-check after publishing and fall through to discovery instead. reclaim installs an entry a reaper reserved after its check -- the warm entry is still visible during the stop, and the reaper's claim succeeds because reclaim's own take() just made the lease ours. Re-check likewise. the reservation was released before the entry was removed -- the pop belonged to the caller, leaving a gap where the container is stopped, the entry is still in `_warm_pool`, and nothing marks it. `_destroy_warm_entry` removes it itself, inside the reservation; the pop stays deferred relative to the stop, just not to the reservation. reconcile adopts a container this instance is tearing down -- adoption is a promote and needs the same reservation check as the others. Neither existing guard excludes it: the claim succeeds because the lease is ours, and on `memory` the recovery grace is skipped outright. The pre-round-trip checks in reuse and reclaim are kept as early-outs, since they skip a health check and a store round trip on a doomed entry, and are pinned to that job rather than to a correctness role they no longer hold. The teardown reservation predicate runs under `_lock`, so it must not touch the lock. Documented rather than engineered around: making the lock reentrant to tolerate it would trade a loud hang for a quiet class of re-entrancy bugs across the rest of the provider. * fix(sandbox): honor local teardown after ownership publish * fix(sandbox): clear a stale warm entry when an id becomes active Active and warm are exclusive states, and the two register paths were the only place that could hold both: they inserted into `_sandboxes` without popping `_warm_pool`, so one container ended up with two reapers. `_reap_expired_warm` judges an entry by its warm timestamp and never consults `_last_activity`, so it stops a container an agent is actively using while `_sandboxes` still hands out its client. Reachable because `_reconcile_orphans` adopts an untracked-but-running container into the warm pool inside the register's publish -> track window, and on the `memory` store it adopts on sight: `_adoptable_after_grace` short-circuits when `supports_cross_process` is False, so an id carrying this process's own lease reads as adoptable. That window is new to this branch -- on main the track was a single locked insert with nothing before it. Both register paths now pop the warm entry inside the same locked section that installs the active one. * fix(sandbox): harden ownership renewal teardown --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Daoyuan Li
|
75fa028e89
|
fix(artifacts): serve inline binary artifacts via FileResponse for Range support (#4281)
* fix(artifacts): serve inline binary artifacts via FileResponse for Range support Audio/video/image artifacts that are previewed inline (not downloaded, not active content) were served by reading the whole file into memory and returning it through a plain Response. That response never sets Accept-Ranges and ignores any Range header the browser sends, so seeking an <audio>/<video> element backed by this endpoint always gets the full 200 response back instead of a 206 partial response for the requested byte range -- which is why dragging the seek bar on an audio artifact restarts playback from the beginning instead of jumping to the new position. Route this case through FileResponse instead (as the active-content/ download branch already does), which handles Range/If-Range natively. Verified with a real Range request against the endpoint: initial load now reports Accept-Ranges: bytes, and a ranged GET returns 206 with the correct Content-Range and only the requested slice of bytes. Fixes #3240 * fix(artifacts): address review nits on the inline FileResponse branch Two non-blocking nits from review: - Drop the redundant filename= kwarg on the inline_file FileResponse call. Content-Disposition is already set explicitly on this branch, and FileResponse only uses filename to setdefault that same header -- which is a no-op once it's already present. Harmless today, but removes the latent risk that a future Starlette version turning that setdefault into a hard set would silently flip inline preview to attachment. - Reword the blocking-IO regression-anchor docstring: it claimed awaiting get_artifact for a binary artifact does "zero filesystem IO", but _read_artifact_payload still runs exists/is_file/ mimetypes.guess_type/is_text_file_by_content (an 8 KB read) for binary files too, just offloaded via asyncio.to_thread like the text branch. The gate has nothing to catch because that IO is off the event loop, not because there's none -- reworded to say no full-file read happens, matching the accurate framing already used one paragraph up. tests/test_artifacts_router.py, tests/blocking_io/test_artifacts_router.py (19 tests) are green, and ruff check/format are clean on both changed files. |
||
|
Aari
|
a0e1d82ef4
|
fix(workspace-changes): offload blocking filesystem IO in text-cache lifecycle (#4268)
* fix(workspace-changes): offload blocking filesystem IO in text-cache lifecycle capture_workspace_snapshot and record_workspace_changes offload their scans via asyncio.to_thread, but ran the snapshot text cache's whole lifecycle on the event loop: roots resolution (os.path.abspath), tempfile.mkdtemp, and shutil.rmtree on both the capture-failure branch and record_workspace_changes' finally. That finally runs on every agent run, including abort paths, so each run removed up to max_files cached texts on the loop. Offload the roots + mkdtemp prep through one _prepare_capture worker hop, and route both rmtree call sites through _remove_text_cache_dir. Cleanup stays best-effort: it swallows and logs, so a failing cleanup cannot replace the exception or result already in flight. asyncio.shield is deliberately not used -- to_thread submits to the pool immediately, so cancelling the future does not stop the running thread and the cache is still removed under single and repeated cancellation. Externally observable behavior is unchanged. Found via `make detect-blocking-io`: these were the last 2 HIGH findings in the repo, which now reports none. The roots resolution is invisible to that scanner (sync helper, cross-file call) but blocks the same async path, and the anchor cannot reach the rmtree without it. Add tests/blocking_io/test_workspace_changes_recorder.py, driving the capture-failure branch and the record finally. Teeth verified per clause under the strict Blockbuster gate: reverting each offload alone reddens its own call (os.path.samestat for rmtree, os.path.abspath for roots/mkdtemp). * fix(workspace-changes): make text-cache prepare handoff cancellation-safe _prepare_capture creates the mkdtemp cache dir inside the to_thread worker, so a run cancelled after mkdtemp but before the coroutine receives the path orphaned the dir. Shield the prepare future and, on cancellation, reclaim its result to remove the dir before re-raising. mkdtemp stays offloaded (the blocking-io gate flags os.mkdir from deerflow code). Adds a deterministic cancellation regression. * fix(workspace-changes): drain repeated cancellation in text-cache reclaim The mkdtemp handoff guard reclaimed the shielded worker's result on the first CancelledError, but the reclaim await was itself cancellable: a second cancel landed there, slipped past `except Exception` (CancelledError is BaseException), and skipped the reclaim while the shielded worker still finished — orphaning the deerflow-workspace-changes-* dir. Repeated cancelled runs accumulate leaks. Move reclaim+remove into a task the caller cannot abandon and drain repeated cancellation until it completes, then restore the cancellation. A repeat cancel interrupts the await, not the task, so the dir is never abandoned; the loop exits only once cleanup has finished, leaving no pending task. Non-cancel paths are unchanged. Adds test_capture_workspace_snapshot_repeated_cancellation_leaks_no_text_cache (double-cancel regression). make test-blocking-io: 43 passed. |
||
|
Huixin615
|
c9b6131f8f
|
fix(skills): reload mounted skills without restarting Gateway (#4264)
* fix(skills): add admin-only reload endpoint * fix(skills): preserve cache when reload fails |
||
|
AochenShen99
|
94a34f382d
|
feat(context): record effective memory identity per run (#3556)
* feat(context): record effective memory identity per run * fix(context): address memory identity review feedback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
ly-wang19
|
289adcbb02
|
fix(mcp): offload blocking filesystem IO in MCP config update (#3552)
* fix(mcp): offload blocking filesystem IO in MCP config update update_mcp_configuration resolved the extensions config path, probed its existence, read the raw JSON, wrote the merged config, and reloaded it — all blocking filesystem IO on the event loop (PUT /api/mcp/config). The whole read-modify-write after the async admin check has no interleaved awaits, so it moves into one _apply_mcp_config_update helper dispatched via asyncio.to_thread; the masked response is built on the loop. The secret-preserving merge, error codes, and the stdio command allowlist are unchanged. Found via `make detect-blocking-io`. Same class as #3457 / #3529 / #3551. Add tests/blocking_io/test_mcp_router.py anchor, verified red->green under the strict Blockbuster gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): serialize concurrent config updates with a write lock Address review on #3552: offloading the read-modify-write to a worker thread dropped the implicit serialization the single-threaded event loop provided, so two concurrent PUT /api/mcp/config calls could interleave and clobber each other. Guard the offloaded RMW with a module-level asyncio.Lock to restore within-process atomicity (cross-process writers remain a separate, pre-existing concern). Add a serialization regression test (red->green: without the lock the tracked max concurrency exceeds 1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address mcp config blocking io review --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
AnoobFeng
|
446fa03801
|
fix(context): resolve context compress bug (#4065)
* fix(runtime): persist original human input outside model sanitization * refactor(history): load thread messages by global event sequence * fix(frontend): make summarization rescue a transient history bridge * fix(frontend): old message not append tail 1. add identity anchor 2. add bridgeOrder * fix(frontend): lint error fix * fix: address review feedback and harden pagination coverage - defer transient history ref writes until after render commit - cover large middleware-only history scans - verify infinite-query refetch recalculates page cursors - document AI event types and anchor-weaving differences * fix: harden message pagination and enrichment - append unmatched live tails after canonical history - warn and stop when pagination has_more lacks a cursor - deep-copy restored UI messages to isolate model-facing content - log invalid event sequence and non-advancing cursor errors - pass user_id explicitly through event-store history queries - cover middleware-only AI runs across memory, JSONL, and DB stores * fix: address pagination review feedback * fix(frontend): checkpoint has unknow redener content, optimize the anchor policy * fix(frontend): unit test issue missed previously, remove the TanStack cache trimming * fix(gateway): harden message history queries and provenance - reject externally forged original_user_content metadata - validate provenance metadata in upload and sanitization middleware - make run lookups fail closed by default - batch feedback queries by run ID - align memory message filtering with persistent stores |
||
|
AochenShen99
|
658c39ccf7
|
feat(skills): Add native SkillScan phase 1 for skills (#3033)
* Add phase 1 skill static scanning * Rework SkillScan phase 1 as native scanner * refactor(skillscan): align phase 1 with trimmed RFC contract - SecurityFinding: 7 fields (rule_id, severity, file, line, message, remediation, evidence); category/analyzer derive from the rule_id prefix, confidence/column/fingerprint/metadata removed - scan_archive_preflight()/scan_skill_dir() are pure functions: no ScanContext, no policy schema; CRITICAL-blocks is a code constant and skill_scan.enabled is applied by enforce_static_scan()/callers - secret-* evidence is redacted before findings leave the scanner - de-dup keys on (rule_id, file, line) so repeated occurrences keep distinct locations for agent self-correction - cloud-metadata detection consolidated into network-cloud-metadata - nested zip members get a one-level stdlib magic-byte peek; an executable member escalates package-nested-archive to CRITICAL - install metadata sidecar removed (Phase 7 decides if it is needed) - rule specs moved next to their analyzers; skillscan/rules/ removed - tests updated + new anchors: redaction, dedup lines, nested-zip escalation, single cloud-metadata rule, bundled-skill zero-CRITICAL * fix(skillscan): tighten reverse-shell/secret/archive scan rules from review Address PR #3033 review feedback on the native SkillScan analyzers: - Reverse-shell false positives: split shell detection by signal strength (/dev/tcp/, nc -e stay CRITICAL; bash -i, mkfifo -> new HIGH shell-reverse-shell-heuristic, warn->LLM). The Python check is now AST-anchored on real socket.socket/os.dup2/subprocess call sites instead of raw-text substring matching, so prose/docstrings no longer hard-block. - Secret evidence: _redact_secret_evidence returns [redacted] with no secret bytes (was value[:6], which leaked 2 real token bytes past the prefix). - Archive DoS: cap outer archive member count (_MAX_ARCHIVE_MEMBERS=4096); scan_archive_preflight early-aborts with a package-too-many-members CRITICAL finding (routes through the existing blocked->400 fail-closed path). - shell-destructive-command: broaden the rm -rf matcher to sensitive system roots (/home, /usr, /*, --no-preserve-root /) while leaving safe subpaths unflagged. - Dead code: collapse _decode_text_for_analysis to a single decode path and drop the unused _TEXT_SUFFIXES set and _has_text_shebang helper. - local_skill_storage: document why the host_path branch keeps app_config possibly-None (lazy kill-switch resolution; avoids eager get_app_config in config-free environments such as CI). Tests: new negative/positive coverage in test_skillscan_native.py. Full backend suite 6616 passed, 26 skipped. |
||
|
hataa
|
b85c672cc1
|
fix(channels): offload blocking filesystem IO in Wechat channel (#3925)
WechatChannel made synchronous filesystem calls (mkdir, write_text, read_bytes, Path.replace, unlink) directly inside async entry points: _poll_loop, _bind_via_qrcode, _ensure_authenticated, _extract_image_file, _extract_file_item, start, _send_image_attachment, _send_file_attachment. Under slow disks, large files, or concurrent load these blocked the asyncio event loop and stalled the channel worker. Construction was also blocking: __init__ called _load_state() (os.stat + read_text) synchronously, and ChannelService._start_channel() instantiates the channel directly on the async path, so constructing WechatChannel in an async context raised BlockingError. Persisted state (auth token + cursor) is now loaded in start() via asyncio.to_thread, leaving __init__ IO-free. Offload each call to a thread via asyncio.to_thread, matching the existing pattern in channels/manager.py and dingtalk.py. The sync helpers (_save_state, _save_auth_state, _load_auth_state, _stage_downloaded_file) keep their signatures; only the async call sites wrap them. Adds tests/blocking_io/test_wechat_channel_state.py as a regression anchor covering the IO-free constructor (the production _start_channel path), the staging write path, and the auth-state read path. Detected by `make detect-blocking-io`. |
||
|
AnoobFeng
|
5acd0b3ba8
|
fix(gateway): offload gateway upload file IO (#3935)
* fix(gateway): offload gateway upload file IO Move Gateway upload router filesystem work off the asyncio event loop by using a dedicated ContextVar-preserving file IO executor. Use async sandbox acquisition for non-mounted sandbox uploads and offload remote sandbox sync together with host file reads. Add blocking-IO regression coverage for upload, list, delete, and remote sandbox sync paths. * fix(gateway): align file IO worker env var prefix Rename the file IO executor worker-count environment variable from DEERFLOW_FILE_IO_WORKERS to DEER_FLOW_FILE_IO_WORKERS to match the repo's existing runtime configuration prefix convention. |
||
|
hataa
|
e9161ff148
|
fix(channels): offload blocking filesystem IO in Discord channel (#3927)
DiscordChannel ran synchronous filesystem IO on the event loop: thread-mapping persistence/restore and outbound attachment reads. Offload all of it via asyncio.to_thread: - start() -> _load_active_threads (restore mappings on startup) - _on_message -> _persist_thread_mappings (flush mappings to disk) - send_file -> _read_attachment_bytes (read bytes; handed to discord.File as an in-memory BytesIO buffer) Thread-mapping state is split to avoid a race surfaced in review (#3927): _record_thread_mapping updates the in-memory _active_threads dict and _active_thread_ids set synchronously on the event loop, so a follow-up message in a newly created thread is recognized immediately — before the offloaded persistence write completes. Deferring that update into the worker thread opened a window where _on_message's membership check misclassified the message as orphaned and created a duplicate thread. __init__ only computes paths, so construction stays IO-free. Blockbuster regression tests cover the IO-free constructor, the record-then-persist split (memory visible before persistence), discard of a replaced thread id, and the load path. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
AnoobFeng
|
debb0fd161
|
feat(persistence): wire alembic migrations, bootstrap schema on startup (#3706)
* feat(persistence): wire alembic migrations + bootstrap schema on startup Closes #3682. Pre-#3658 DBs lack the `runs.token_usage_by_model` column because alembic was never wired up — startup only ran `create_all`, which never ALTERs existing tables. Adds a hybrid bootstrap in FastAPI lifespan (replaces bare `create_all`): - empty DB → create_all + stamp head - legacy DB → stamp 0001_baseline + upgrade head - versioned DB → upgrade head Concurrency: Postgres `pg_advisory_lock` (cross-process); SQLite per-engine `asyncio.Lock` + 30s `PRAGMA busy_timeout` on both prod and alembic engines. Column revisions use `safe_add_column` / `safe_drop_column` idempotent helpers as fallback. Other bits: - 0001 baseline (chain root) + 0002 add `runs.token_usage_by_model` - `include_object` filter so alembic ignores LangGraph checkpointer tables - `make migrate-rev MSG="..."` for authoring new revisions (no migrate/stamp targets — startup is the only execution path) - Tests: three-branch decision, concurrency, #3682 regression, env filter, blocking-IO gate anchor - CLAUDE.md: new "Schema migrations" section * fix(style): fix lint error * perf(persistence): address review feedback on alembic bootstrap Behavioural fixes - _SQLITE_LOCKS now keyed via WeakKeyDictionary so id-reuse after GC cannot return a stale, loop-bound lock and the cache cannot leak one entry per disposed engine. - safe_add_column compares nullable / server_default against the desired column when the name already exists and emits a warning on drift, surfacing manual-ALTER workarounds instead of silently no-op'ing. - _postgres_lock issues SET LOCAL idle_in_transaction_session_timeout=0 before pg_advisory_lock, so managed Postgres cannot kill the idle lock-holding session mid-upgrade and silently release the advisory lock. - legacy branch now backfills missing baseline tables via a restricted create_all (Base.metadata.create_all scoped to _BASELINE_TABLE_NAMES). Restores pre-#1930 upgraders whose channel_* tables were never provisioned, without pre-empting future create_table revisions for newly-added models. Schema parity - runs.token_usage_by_model gains server_default=text("'{}'") in both the ORM model and the 0001_baseline create_table, matching what 0002 adds via ALTER. create_all and alembic-upgrade paths now produce identical column definitions. - New parity test compares Base.metadata.create_all output against a pure alembic upgrade base->head, asserting column-set, nullable, and server_default agree across all tables (normalized through the same helper safe_add_column's drift check uses). Guards - test_baseline_table_names_constant_matches_0001 pins _BASELINE_TABLE_NAMES to 0001_baseline.upgrade()'s actual output -- the constant cannot drift silently when someone edits 0001. - test_legacy_backfill_skips_non_baseline_tables verifies the restricted backfill does not create a phantom table on Base.metadata, modelling a future revision that would otherwise collide on op.create_table. Doc residuals - Three-branch decision table is now consistent across bootstrap.py top docstring, engine.py comment, test module docstring, and CLAUDE.md. - Stale test anchor in blocking_io/test_persistence_engine_sqlite.py docstring now points at the real file. * fix(style): fix lint error * fix(persistence): close drift detection holes - _check_column_drift compares column type via a family equivalence allowlist ({JSON, JSONB}). Catches the wrong-type workaround `TEXT NOT NULL DEFAULT '{}'` that previously slipped through silently, while keeping Postgres JSON/JSONB dialect reflection quiet. Reflected and desired type are also echoed in every drift warning's payload for operator triage. - Extract _escape_url_for_alembic so bootstrap._alembic_safe_url and scripts/_autogen_revision share the ConfigParser % escape rule instead of duplicating it. - backend/README.md: add `make migrate-rev MSG=...` to Commands and a Schema Migrations section per the repo's README/CLAUDE.md sync policy. - test_base_to_dict.py: scope the test ORM class to an isolated MetaData so the create_all-vs-alembic parity test (added in the previous commit) is not polluted by the phantom table on the full pytest session. |
||
|
ly-wang19
|
435edbd8c6
|
fix(artifacts): offload blocking filesystem IO in artifact serving (#3551)
get_artifact ran its filesystem work directly on the event loop: virtual-path resolution (os.path.abspath via .resolve()), exists/is_file probes, MIME sniffing (mimetypes lazily stats the system MIME DB on first use), full-file read_text/read_bytes, is_text_file_by_content (open+read), and .skill ZIP open+extract. So serving any artifact blocked the loop for the whole read; `make detect-blocking-io` flagged it. Same class as #3457 / #3529. Offload each branch's IO via asyncio.to_thread: one sync helper per branch (_load_skill_archive_member, _read_artifact_payload) folds stat + MIME + read / extract into a single worker hop and returns a small (kind, mime, payload) plan the handler turns into the response on the loop. FileResponse (download / active content) keeps streaming the file itself. Behavior, branching, error codes, and security boundaries are unchanged. Add tests/blocking_io/test_artifacts_router.py anchor (text / binary / .skill member), verified red->green under the strict Blockbuster gate. The gate also caught a blocking call the static scan missed: resolve_thread_virtual_path's .resolve() (os.path.abspath), now offloaded too. Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ly-wang19
|
a09f9668a5
|
fix(persistence): offload sqlite dir creation off the lifespan event loop (#3574)
init_engine runs on the FastAPI lifespan event loop, but created the SQLite data directory with a synchronous os.makedirs (a stat + mkdir syscall), blocking startup. Dispatch it via asyncio.to_thread, mirroring the #1912 fix for the checkpointer's ensure_sqlite_parent_dir. Adds a Blockbuster-gated regression test in tests/blocking_io/ that drives the real init_engine path with a not-yet-existing sqlite_dir; it trips BlockingError if the makedirs regresses onto the event loop. Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> |
||
|
Nan Gao
|
2b301e8211
|
fix(channels): harden runtime credential management APIs (#3581)
* fix(channels): harden runtime credential management APIs * fix(channels): address review feedback on credential hardening Follow-up to the runtime credential-hardening pass, resolving five review findings: - WeChat auth persistence now writes through a 0o600 NamedTemporaryFile + Path.replace instead of write_text-then-chmod, so the iLink bot_token is never briefly readable at umask defaults (mirrors ChannelRuntimeConfigStore). - The post-write chmod is split into its own try/except: a chmod failure on a filesystem without POSIX perms now logs at debug instead of masquerading as a "failed to persist" warning. - Extracted the three near-identical _require_admin_user helpers (mcp, channel_connections, channels) into a single require_admin_user(request, *, detail) in app/gateway/deps.py; each router supplies its own detail string. - Strengthened the runtime-config-store chmod coverage: a new test injects a temp-file chmod failure and asserts it is logged at debug while the destination is still owner-only (mutation-verified to fail if the chmod is dropped), plus a loose-pre-existing-file case. - Removed the unused _FakeRepo from the blocking-io test: its isinstance gate routes through the repo-less 503 path, so neither stub was ever invoked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |