mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-26 16:07:53 +00:00
8 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Daoyuan Li
|
40c4ec32f4
|
fix(blocking-io): trace self/cls attribute chains and local aliases in the call graph (#4200)
* fix(blocking-io): trace self/cls attribute chains and local aliases in the call graph
_record_call_ref only recorded a call-graph edge for bare-name calls and
literal self./cls. single-hop attribute calls (self.flush()). Any other
receiver shape fell through the "." not in call_name fallback and was
silently dropped from the graph -- including a deeper self./cls.
attribute chain (self.store.flush()), a local variable holding a
self./cls. attribute (store = self.store; store.flush()), or a
parameter used directly as a receiver. A real blocking call reachable
from async code only through one of those shapes never surfaced as a
finding, the opposite (and more dangerous) failure mode from the
duplicate-helper-name over-report this detector already documents.
Trace those shapes back to a self./cls. attribute or a parameter,
within the same function only, and resolve them through the same
bare-method-name fallback already used for receivers that cannot be
resolved to a name at all -- no new false-positive risk beyond what
that existing fallback already accepts.
* fix(blocking-io): narrow alias tracking to fix three scope-creep bugs
The alias/receiver tracking this detector added reused dotted_name(),
which intentionally unwraps ast.Call/ast.Subscript for blocking-call
pattern matching elsewhere in this module. Reusing it for alias
extraction let a Call or Subscript result inherit its base's
alias-worthiness, so factory().flush(), client = factory(); client.flush(),
and client = clients[0]; client.flush() were all incorrectly treated as
calls on a traced receiver. Add _simple_receiver_name(), a restricted
Name/Attribute-only extractor, and use it wherever a receiver/alias is
extracted instead of dotted_name().
Alias state also only ever grew: _record_local_receiver_alias_targets
never removed a name once traced, so a later reassignment to a
non-traceable value (client = NonBlockingClient()) left the name
aliased forever, still exposing unrelated same-named methods.
Reassigning now resolves traceability from scratch and kills the name
when the new value isn't traceable.
Separately, if/else branches had no isolation: with no visit_If
override, body and orelse shared one mutable alias set, so an alias
added in one leaked into the other and the result depended on which
branch was textually first. Add a visit_If override that snapshots
aliases before the branch, resets between body and orelse, and unions
their exit states afterwards -- a conservative, order-independent
may-alias join. Scoped to ast.If only; ast.Try/ast.Match keep the
previous unisolated traversal (different, more complex control-flow
semantics, out of scope here).
Finally, _visit_function pushed the new function's context before
visiting decorator_list/args/returns, but those expressions run at
definition time in the enclosing scope, not the function body. A
default value referencing an outer name that happens to match one of
the function's own parameter names (receiver = Store(); async def
route(receiver=receiver.flush())) was misattributed to route itself.
Visit decorators, parameter defaults/annotations, the return
annotation, and PEP 695 type-parameter bounds before pushing the new
function's context so they resolve against the enclosing scope.
Real-scanner output against the actual backend tree is unchanged
(41/41 findings, byte-identical JSON) -- these were latent
false-positive/negative risks in shapes the current codebase doesn't
happen to contain, not active miscounts.
* fix(blocking-io): fix three more alias-tracking and definition-time bugs
_record_local_receiver_alias_targets ran before the assignment's own
value was visited, so an assignment's RHS was analyzed against the
alias state *after* the target had already been updated/killed for
this same statement. Python evaluates the RHS before binding the
target: with `client = self.store` followed by
`client = client.flush()`, the second statement's target update killed
`client`'s alias before its own RHS (`client.flush()`) was visited, so
that call silently disappeared from the graph. visit_Assign and
visit_AnnAssign now visit the RHS first and only update the target's
alias afterward, matching Python's own evaluate-then-bind order.
_simple_receiver_name still returned the trailing attribute name
whenever its recursive parent lookup came back unsupported (a Call or
Subscript), instead of refusing the whole chain -- so `factory().client`
and `clients[0].client` both collapsed to plain "client", which, when
"client" was also a traced parameter or local alias, incorrectly linked
`factory().client.flush()` to an unrelated same-file `Store.flush`.
Return None instead of falling back to `node.attr`, so an unsupported
node anywhere in the chain makes the whole receiver unresolved rather
than a truncated suffix of it.
Finally, _visit_function's enclosing-scope walk of decorators,
defaults, annotations, and type_params recursed into every
subexpression uniformly, including ones that don't actually execute at
definition time: a lambda's body, a bare generator expression's
element/later-for clauses, annotations postponed by `from __future__
import annotations`, and PEP 695 type-parameter bounds (always
evaluated lazily, in their own hidden function, only if something like
T.__bound__ is ever accessed). Add visit_Lambda/visit_GeneratorExp
overrides that stop at exactly the eager subset (a lambda's own
parameter defaults; a generator's outermost iterable), skip parameter/
return annotations entirely once a `postponed_annotations` flag is set
by the future import, and drop the type_params walk instead of moving
it to the enclosing scope.
Real-scanner output against the actual backend tree is unchanged
(41/41 findings, byte-identical JSON) -- these were latent risks in
shapes the current codebase doesn't happen to contain, not active
miscounts.
* fix(blocking-io): preserve eager traversal for immediately invoked lambdas and consumed generators
visit_Lambda/visit_GeneratorExp (added last round to stop treating merely
created lambda/generator objects as executing at definition time) were
unconditional, so they also suppressed bodies that genuinely execute right
away: an immediately invoked lambda ((lambda: ...)()) and a generator
expression passed directly to an eager-consuming builtin (list/set/tuple/
frozenset/dict/sorted).
visit_Call now marks a Lambda used as its own func, or a GeneratorExp passed
as the sole argument to one of those builtins, by node identity before
generic_visit runs. visit_Lambda/visit_GeneratorExp check that marker and,
on a match, visit the node fully instead of applying the lazy walk. A
lambda/generator that is merely created, stored, passed as a callback, or
invoked later through a variable is unaffected and stays lazy.
* fix(blocking-io): scope lambda/generator laziness to definition-time expressions only
visit_Lambda/visit_GeneratorExp were unconditional overrides, so they
suppressed lambda bodies and generator elements everywhere the visitor
reached one, not only inside another function's definition-time
expressions (decorators, parameter defaults/annotations, return
annotation) where that suppression is actually needed. In ordinary
function-body code this caused real false negatives: a lambda stored in
a local and called through that name (callback = lambda: os.listdir(".");
callback()), a generator reduced by sum/any/all/min/max, a bare
lambda/generator that is merely created, and a generator wrapped in
another lazy iterator like map(...) all went unscanned, even though none
of them are definition-time expressions at all.
The previous fix for this (an id()-keyed marker set covering exactly two
eager shapes -- an immediately invoked lambda, and a generator passed
directly to a fixed list of eager-consuming builtins) narrowed the
suppression back down, but only for those two shapes, and the underlying
eager-consumer builtin set itself excluded true reducers (sum/any/all/
min/max) that consume their generator argument just as eagerly as list/
set/etc. Both are instances of the same problem: enumerating every shape
in which a lambda or generator happens to be invoked/consumed piecemeal
inside an AST visitor, which is unbounded in the general case.
Replace both mechanisms with a single boolean,
_in_definition_time_expression, set only while _visit_function walks
another function's own decorators/defaults/annotations/return
annotation. visit_Lambda/visit_GeneratorExp apply their lazy
(defaults-only/outermost-iterable-only) walk only while it is set;
everywhere else they fall through to a full generic_visit, scanning
lambda bodies and generator elements unconditionally -- the same
conservative, over-report-rather-than-infer stance this file already
takes for reachability elsewhere.
This removes EAGER_ITERABLE_CONSUMER_NAMES and the two identity-marker
sets entirely rather than growing them further. The one shape this
newly gives up on -- an immediately invoked lambda or eagerly consumed
generator used as another function's decorator/default/annotation value
-- is now an explicit, narrow, documented limitation (see
backend/AGENTS.md): definition-time expressions never scan a nested
lambda body or generator element, full stop, regardless of whether it
happens to be invoked right there.
Targeted suite (test_detect_blocking_io_static.py +
test_scan_changed_blocking_io.py + test_detector_repo_root.py +
blocking_io/test_gate_smoke.py): 65/66 pass, the one failure a
pre-existing Windows path-separator comparison unrelated to this file.
Full backend suite: identical 64 pre-existing failures on both the
pre-fix and post-fix commit, confirmed by diffing the two failure lists
directly -- zero regressions. Real scanner against the actual backend
tree: 41/41, byte-identical JSON before and after -- these were latent
risks in shapes the current codebase doesn't happen to contain, not
active miscounts.
|
||
|
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. |
||
|
AochenShen99
|
a838546a2b
|
chore(blocking-io): fail-loud repo-root resolution and shared detector CLI shim (#3512)
* chore(blocking-io): fail-loud repo-root resolution and shared detector CLI shim The three detectors resolved REPO_ROOT with depth-indexed Path(__file__).resolve().parents[4]. If a detector file ever moves to a different directory depth, scan roots resolve under the wrong directory and the detector reports zero findings with no error — a silent-zero failure shape for a detection tool. - Add support/detectors/repo_root.py: resolve the repo root by walking upward to the .git marker (checked with exists() so git worktrees, where .git is a file, also resolve), raising RuntimeError when no marker is found. All three detectors use it at import time, so a relocated detector fails loudly instead of scanning an empty tree. - Extract scripts/_detector_cli.py from the three character-identical CLI shims; the sys.path computation lives in one place and raises when backend/tests cannot be found. - tests/test_detector_repo_root.py pins: resolution from an unmarked location raises instead of returning an empty scan; all three detectors share the resolved root; each CLI shim delegates to its detector. Testing: backend `make test` (4278 passed); smoke-ran `make detect-blocking-io`, `make detect-thread-boundaries`, and `scripts/scan_changed_blocking_io.py --base upstream/main`. Closes #3510 (review follow-up to #3503). * chore(blocking-io): declare detector modules import-only, drop script-mode residue Adversarial review caught that blocking_io_static.py and thread_boundaries.py kept shebangs and __main__ blocks but can no longer run as plain scripts: the new `from support.detectors.repo_root import` executes before anything puts backend/tests on sys.path, so direct invocation dies with ModuleNotFoundError before argparse. Direct execution was never a documented entry point (Makefile targets, the scripts/ shims, the blocking-io-guard skill, and tests all go through the support.detectors package), so converge on import-only instead of re-adding per-module bootstrap: drop the shebangs and the now unreachable __main__ blocks (plus the `import sys` they kept alive) and state the supported entry points in each module docstring. The shim delegation tests in test_detector_repo_root.py pin the supported CLI paths. Testing: backend `make test` (4278 passed); `make detect-blocking-io` and `make detect-thread-boundaries` smoke-ran. |
||
|
AochenShen99
|
dc2ababf00
|
feat(skill): add blocking-io-guard — SOP skill for blocking-IO triage and runtime anchors (#3503)
* feat(blocking-io): add changed-lines blocking-IO scanner (L1) * feat(blocking-io): add scan-changed CLI wrapper * feat(skill): add blocking-io-guard developer SOP skill * docs(blocking-io): point contributors at the blocking-io-guard skill * style(blocking-io): apply ruff format to scanner and tests * docs(backend): document changed-lines blocking-IO scanner in CLAUDE.md * feat(skill): add post-fix re-scan check and PR batching policy * refactor(skill): fix SOP step ordering, align template with repo conventions - Move re-scan into an explicit 'apply the fix' step (was wedged after anchor generation while telling you to go back before the anchor) - Renumber steps 0-6; drop undefined 'L1' jargon - Mode A: document that the diff is <base>...HEAD (commit first) - Mode B: prefer make detect-blocking-io + findings JSON file - anchor template: module-level pytestmark per tests/blocking_io convention - CLAUDE.md: fix 'git diff --base' phrasing * fix(skill): catch findings introduced without touching the blocking line Review follow-up: changed-line intersection alone misses the case where a new async caller exposes an old sync helper — the static finding sits on the untouched blocking line, so Mode A returned empty and the SOP stopped on a false 'no blocking-IO surface'. Selection is now a union over the changed files: - findings on added lines of git diff <base>...HEAD (kept: a second identical symbol in an already-flagged function collides on the stable key and only this selection sees it); - findings new versus the merge base, matched by (path, function, symbol) — never line numbers. Base sources are materialized via git show <merge-base>:<path>; files absent at base count every head finding as new. SKILL.md now states the residual same-file-only blind spot (cross-file async callers) instead of treating an empty list as proof of zero exposure, and only requires reading sop-skeleton.md when generalizing to another detector domain. * docs(skill): examples teach test-writing, the teeth check defines the rule All examples in the references/template are filesystem-flavored; make explicit that they are instances, not the SOP's boundary — the same rules apply to every detector category (FILE_IO, HTTP, SUBPROCESS, SLEEP) and acceptance is always red/green teeth, never similarity to an example. Neutralize the template's arrange comment accordingly. * fix(blocking-io): harden changed-lines scanner per review - Dedup the union selection by the stable key (path, function, symbol) instead of dict identity, so a future selector returning copied dicts cannot silently empty the result. - parse_changed_lines now handles any unified diff: context lines advance the new-file counter, \-markers and deletions do not, and the counter resets at each +++ header. Previously correct only for --unified=0. - Add blocking_io_static.scan_source (in-memory scan); base-version comparison no longer round-trips through temp files. - Empty Mode A report now prints the same-file-only reachability caveat at the point of use instead of relying on the SOP text alone. * docs(skill): bound best-effort cleanup when the offload sits in finally Lesson from the #3505 review: the SOP routinely drives 'offload the cleanup branch' transformations, and an awaited cleanup in finally can mask or stall the primary exception. One sentence in Step 2 closes that gap at the point where the fix is written. |
||
|
AochenShen99
|
da41701f87
|
Add static blocking IO inventory (#3208)
* feat(detectors): add static blocking IO inventory * refactor(detectors): drop superseded runtime probe; clarify static report path - Remove the #2924 custom runtime blocking IO probe entirely: backend/tests/support/detectors/blocking_io.py, backend/tests/test_blocking_io_detector.py, backend/tests/test_blocking_io_probe_integration.py, and the pytest_addoption / pytest_runtest_call / pytest_runtest_teardown / pytest_sessionfinish / pytest_terminal_summary hooks plus the blocking_io_detector fixture from backend/tests/conftest.py. Its narrow DEFAULT_BLOCKING_CALL_SPECS (time.sleep, requests, httpx, os.walk, Path.resolve, Path.read_text, Path.write_text) cannot serve as a CI gate; a Blockbuster-backed runtime detector will land in a separate follow-up PR. Leaving the half-coverage probe alongside the static inventory in this PR added a redundant detect path with no production value. - Address Copilot review comments on backend/README.md and backend/CLAUDE.md by stating explicitly that the JSON report writes to .deer-flow/blocking-io-findings.json at the repository root, whether the target is invoked from the repo root or from backend/. Verified: pytest tests/test_detect_blocking_io_static.py (18 passed), ruff check + format on touched files (passed), make detect-blocking-io from both repo root and backend/ produce the same 105-finding report at <repo-root>/.deer-flow/blocking-io-findings.json. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
AochenShen99
|
e344be8d94
|
feat(tests): add Blockbuster runtime gate for event-loop blocking IO (#3229)
* feat(tests): add Blockbuster runtime gate for event-loop blocking IO Adds a strict runtime gate that fails CI when sync blocking IO calls run on the asyncio event loop thread through DeerFlow business code. Components: - backend/tests/support/detectors/blocking_io_runtime.py — Blockbuster context scoped to `app.*` and `deerflow.*` so test infrastructure, pytest internals, and third-party libraries stay silent. - backend/tests/blocking_io/conftest.py — pytest_runtest_protocol hookwrapper that wraps every item (setup + call + teardown) with the strict context. Respects `@pytest.mark.allow_blocking_io` opt-out. - backend/tests/blocking_io/test_skills_load.py — regression anchor for the #1917 fix (asyncio.to_thread offload around LocalSkillStorage.load_skills). - backend/tests/blocking_io/test_sqlite_lifespan.py — regression anchor for the #1912 fix (asyncio.to_thread offload around ensure_sqlite_parent_dir). - backend/tests/blocking_io/test_gate_smoke.py — meta-test asserting the gate actually catches unoffloaded blocking IO and that the `@pytest.mark.allow_blocking_io` opt-out works. - backend/Makefile — `make test-blocking-io` target. - .github/workflows/backend-blocking-io-tests.yml — hard-fail PR gate on ubuntu-latest. Windows matrix deferred to follow-up. Dependencies: - blockbuster>=1.5.26,<1.6 added to dev group. Coverage boundary (called out in PR body): the gate only catches blocking IO on code paths the test suite actually exercises. Static AST inventory (separate, informational) is the complementary coverage tool. Three blind spot categories — untested paths, mocked-away paths, env-mismatched paths — are documented in the PR description. Findings surfaced while authoring this PR: - resolve_sqlite_conn_str in runtime/store/_sqlite_utils.py:19 does sync Path.resolve() -> os.path.abspath on the lifespan loop thread, ahead of the #1912 fix. Not addressed here; tracked as follow-up. Tests: 4 passed locally (`make test-blocking-io`). Lint/format: clean (`ruff check` and `ruff format --check`). * fix(tests): scope Blockbuster gate to blocking-io suite * fix(tests): harden Blockbuster runtime gate * test(blocking-io): add project rule extension point * test(blocking-io): address review cleanup |
||
|
AochenShen99
|
0c22349029
|
chore(dev): add async/thread boundary detector (#2936)
* chore(dev): add thread boundary detector * chore(dev): reduce thread boundary detector false positives |
||
|
AochenShen99
|
6e8e6a969b
|
test: add blocking IO detector (#2924)
* test: add blocking IO detector * test: add blocking IO probe option * test: harden blocking IO probe lifecycle * test: move blocking io detector to support |