852 Commits

Author	SHA1	Message	Date
Michael	85ffb66d6e	fix(subagents): stamp UTC-aware datetimes on SubagentResult lifecycle (#5153) ... ## Why DeerFlow declares one timestamp convention in deerflow/utils/time.py: every lifecycle timestamp is UTC (now_iso / datetime.now(UTC)). SubagentResult writers in subagents/executor.py still used naive datetime.now(), so on any non-UTC host the in-memory lifecycle metadata (started_at / completed_at) was local wall-clock time. The sibling durable-batch path (subagents/batch_service.py) already stamps datetime.now(UTC), so the same run model carried two different conventions depending on which path wrote it. ## What changed - Added an executor-local _utcnow() helper that stamps datetime.now(UTC). - SubagentResult.completed_at default in try_set_terminal(), result.started_at in _aexecute(), and the started_at default in _aexecute_admitted() now route through _utcnow(). - Explicit caller-supplied timestamps (completed_at=...) still pass through unchanged. - Added regression tests asserting the default writers produce UTC-aware datetimes. ## Surface area - [x] Backend runtime (deerflow.subagents.executor) - internal dataclass lifecycle metadata, no wire format change - [ ] Frontend UI / Backend API / Sandbox / Skills / Dependencies / Default behavior change ## Bug fix verification - New tests: tests/test_subagent_executor.py::test_timestamp_writers_stamp_utc_aware_datetimes and test_utcnow_helper_returns_utc_aware_datetime encode the convention. - Updated BlockingDateTime.now() in the terminal-publication-order test to mirror datetime.now's optional tz argument. ## Validation - cd backend && python -m pytest tests/test_subagent_executor.py: 136 passed; 2 pre-existing TestBashExecutionHarvest failures reproduce identically on clean main (Windows sandbox env), unrelated to this change. - ruff format + ruff check clean on both changed files. ## AI assistance **Tool(s) used:** Codex (coding agent) **How you used it:** analysis of the timestamp conventions, implementation, and regression tests authored with AI assistance; change reviewed before commit. - [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.	2026-09-03 17:55:15 +08:00
Tu Naichao	ae82f426bf	fix(summarization): stop fraction triggers from crashing the agent build (#4901) ... * fix(summarization): resolve fraction triggers from declared context_window, degrade instead of crashing the agent build A fraction trigger/keep clause requires profile["max_input_tokens"], which any third-party OpenAI-compatible model lacks, so SummarizationMiddleware construction raised ValueError out of create_summarization_middleware and failed the whole agent build (#3103). - factory: translate a declared model context_window into the langchain profile (metadata-only, never reaches the provider payload); explicit caller/override profiles win - summarization factory: drop unusable fraction trigger clauses (absolute clauses survive), fall a fraction keep back to the messages default, and disable compaction with an actionable warning only when no usable trigger clause remains — the agent build never dies from summarization config - docs: config.example.yaml, ModelConfig.context_window, summarization.md * refactor(summarization): share the default keep constant with the fraction fallback The fraction-keep degradation fallback hardcoded ("messages", 20), duplicating SummarizationConfig.keep's default_factory literal. Move the value to a shared DEFAULT_KEEP constant so the two cannot drift apart. * fix(summarization): keep trigger-null + fraction-keep constructing after degradation A trigger of None with a fraction keep hit the all-clauses-dropped branch (has_usable_trigger=False) and disabled compaction, and the accompanying warning claimed configured triggers were all fraction-based when none were configured. Only report nothing-usable when trigger clauses actually existed; trigger:null keeps constructing the never-firing middleware with the degraded keep, matching its behavior outside the degradation path. * fix(summarization): address review — keep manual compaction, validate ContextSize, pin wiring Review follow-ups on #4901: - When every configured trigger is a dropped fraction clause, keep constructing the never-firing middleware (trigger=None) instead of returning None: manual /compact runs with force=True and never consults trigger clauses, so it must keep working for a profile-less model rather than reporting 'compaction is disabled'. The warning now says auto-compaction will not fire while manual compaction remains. - ContextSize gains a config-load validator: fraction values must be in (0,1] (a percent-style 80 instead of 0.8 previously produced a threshold the context could never reach — a silently inert trigger), absolute values must be positive. - New un-monkeypatched integration test pins the shipped wiring (context_window declared -> real factory attaches profile -> fraction clause survives -> middleware constructs), which the stubbed middleware-side tests and kwarg-capturing factory-side tests each stopped short of. - Docs (summarization.md + config.example.yaml) clarify that the fraction resolves against the summary/anchor model's context_window (summarization.model_name when set, else the run model), including the mismatch caveat for a larger-window summary model. * fix(summarization): reject non-finite ContextSize values at config load YAML .nan / .inf pass pydantic's float parsing, and nan <= 0 is False, so the positivity check alone let them through as dead thresholds (count >= nan is always False) — the same silent-inert-trigger class the range validator was added to close. Guard with math.isfinite first, consistent with the existing non-finite guards on mem0 timeout_seconds and poll_after_seconds. * fix(summarization): merge context_window into inferred profile, require whole message counts - construct the model first, then merge max_input_tokens into the provider-inferred langchain profile: passing profile= to the constructor replaced the whole inferred metadata (tool_calling, structured_output, io capabilities, output limits) with the single key. An explicitly configured profile is still never clobbered. - reject non-integral ContextSize values for type=messages at config load: langchain slices the message list with them, so a float index raised TypeError mid-compaction. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-09-03 17:05:09 +08:00
Otavio Rodrigues Santana	27cb73659d	fix(auth): correct OAuth conflict error message + validate multi-worker Postgres claim with real concurrency benchmark (#5026) ... * fix(auth): correct OAuth uniqueness error and index parity on Postgres create_user() caught any IntegrityError on commit and always reported it as a duplicate email. The email pre-check already rules out a real email collision in the common case, so any IntegrityError reaching that handler is actually idx_users_oauth_identity firing instead -- confirmed against both backends: SQLite reports "UNIQUE constraint failed: users.oauth_provider, users.oauth_id", Postgres reports a UniqueViolationError naming the same index. The caller saw "Email already registered" for an OAuth account conflict, which is wrong and would send API consumers debugging the wrong field. Distinguish the two cases via a substring check on the driver error text (both backends name the oauth columns) and raise an accurate message for each. Also add postgresql_where to the same index, alongside the existing sqlite_where. This is not a correctness fix -- verified empirically that Postgres already enforces the same practical uniqueness without it (NULL is never equal to NULL in either backends unique index, so real duplicate (provider, id) pairs are already rejected and NULL/NULL rows are already unconstrained). postgresql_where makes the index genuinely partial on Postgres too, matching the stated intent in the surrounding comment and keeping the index smaller as the common case (plain-password accounts, both columns NULL) accumulates. * test(bench): add multi-process SQLite vs Postgres concurrency benchmark CONFIGURATION.md documents that multi-worker deployments must use Postgres because "SQLite silently ignores row-level locks", but nothing in the repo exercised that claim against real separate worker processes -- the existing checkpoint benchmarks (scripts/benchmark/checkpoint/) measure single-process read/write latency, and the existing Postgres tests (test_pg_schema_integration.py, test_multi_worker_postgres_gate.py) cover schema placement and config validation, not throughput or lock behavior under concurrent load. run_concurrency_bench.py spawns N real OS processes (subprocess.Popen, not asyncio tasks or threads within one process) against the shared users table, mixing reads (get_user_by_email) and writes (create_user) at a configurable ratio, and reports throughput, error counts by exception type, and p50/p95/p99/max latency per run. Measured locally (2/4/8/16 workers, 100 ops/worker, 70/30 read/write): SQLite completed all operations with zero errors at every worker count (busy_timeout absorbs contention rather than raising), but total throughput stayed flat around 28-34 ops/s regardless of worker count, and p99 latency grew from ~400ms at 2 workers to ~5.9s at 16, with a 22s max. Postgres throughput scaled with worker count (41 to 66 ops/s) and p99 stayed under 500ms at every worker count tested. Raw JSON output from both runs is available on request; exact numbers will vary by machine and are not asserted in the test suite. test_bench_concurrency.py unit-tests the pure aggregation logic (percentile math, error grouping, crashed-worker handling) the same way test_bench_checkpoint_channels.py does for the existing benchmarks -- fast, no DB required, not the full multi-process sweep in CI. * fix(auth): inspect the driver exception for OAuth conflict detection str(exc) embeds the full failed INSERT statement, whose column list always names oauth_provider/oauth_id, so a substring check on it misclassified every commit-time IntegrityError on the users table as an OAuth conflict (reproduced on SQLite: a duplicate primary key with a different email raised "OAuth account already linked: None/None"). _is_oauth_identity_violation now inspects exc.orig instead: constraint_name on Postgres, both violated column names present (not a bare "oauth" substring) on SQLite. Also ships the alembic revision idx_users_oauth_identity's postgresql_where predicate needed: 0001_baseline created it as a full index on Postgres, and ORM metadata changes only affect fresh create_all databases, never an already-versioned deployment. Addresses review feedback from willem-bd. * fix(bench): run the concurrency benchmark in an isolated schema and derive paths from the checkout --pg-url accepted an arbitrary database URL while the code pinned postgres_schema="public" and unconditionally ran DELETE FROM users -- against any non-disposable database that permanently destroyed every auth account. Each run now generates a unique throwaway schema (bench_<uuid>), points both the seeder and every worker subprocess at it via postgres_schema, and drops only that schema (DROP SCHEMA ... CASCADE) once the full worker-count sweep finishes. Also stopped hard-coding /opt/deer-flow/backend as the checkout path and .venv/bin/python3 as the interpreter: BACKEND_DIR is now derived from Path(__file__), and workers are spawned with sys.executable (the orchestrator's own interpreter) instead, so the documented uv run python scripts/benchmark/concurrency/run_concurrency_bench.py command works from any checkout. Addresses review feedback from willem-bd. * fix: shorten oauth-index revision id, repin migration-head assertions, fix bench read/write mix - 0017_users_oauth_identity_partial_pg (36 chars) exceeded alembic_version.version_num's VARCHAR(32) limit, which would fail stamping/upgrading on both fresh and existing Postgres deployments. Renamed to 0017_oauth_identity_pg_partial (30 chars). - Repinned every test asserting 0016_subagent_batches as the migration head (test_persistence_bootstrap[.py|_concurrency.py|_regression.py], test_migration_0004/0007/0015) to the new 0017 revision id. - worker.py's `(i % 100) < int(read_ratio * 100)` assumed n_ops >= 100; at the documented default (50 ops/worker, 0.7 read ratio) it produced either all-reads or all-writes, never the claimed mixed workload. Replaced with read_count()/is_read_op(), which distribute an exact round(n_ops * read_ratio) reads evenly across the sequence via modular spacing, and added test_bench_worker.py covering the default values plus small op counts. * fix(bench): establish a real physical connection before timing ops async with sf(): pass entered an empty AsyncSession without checking out a physical connection -- SQLAlchemy stays lazy until the first statement executes. That pushed connection-establishment cost onto each worker's first timed operation instead of conn_time_s, and at 16 workers those 16 cold first-ops (1% of a 1600-op sample) could skew the reported p99. Execute a real `SELECT 1` before starting the timer instead. Verified with a real end-to-end run (uv sync + sqlite backend, 2 workers/10 ops, 0 errors) plus the full auth/bench/migration-bootstrap suites (135 tests) and ruff check/format, all clean. * fix(bench): synchronize workers before timing, fix percentile off-by-one Two remaining measurement issues from review: - run_workers() started the wall clock before spawning any worker, so throughput/wall_time absorbed N processes' staggered Python-startup and connection-establishment cost, and early workers could run ahead of ones still starting. Workers now print READY right before their timed loop and block on stdin for a GO signal; the orchestrator waits for every READY, then starts the timer and releases all workers together. - summarize()'s pct() used int(len(latencies) * p) directly as a zero-based index -- a one-based-rank-as-index bug that put p95 and p99 at the same slot (the max) for any 20-or-fewer-sample run, and for the documented 100-sample default. Now delegates to checkpoint_bench_common.percentile(), the already-correct nearest-rank implementation used elsewhere in the same benchmark family, instead of a second, broken one. Verified: 14/14 unit tests pass (2 new pinned-value regression tests for the percentile bug, using the reviewer's own 20-sample repro), ruff clean, and a real 2/4-worker SQLite multi-process smoke run completes with distinct p95/p99/max latencies and no hang. * fix(bench): absolute SQLite bench path, surface crash diagnostics, exit nonzero on failure; share OAuth index constant + cover Postgres branch Three more findings from review at 5fd25a7: - seed_baseline() cleaned an absolute .deer-flow/bench_data path but handed DatabaseConfig a relative one, which resolves against the CALLER's CWD -- not BACKEND_DIR. Invoking the documented command from anywhere other than backend/ silently pointed the seeder and the (cwd=BACKEND_DIR) workers at two different directories: workers crashed with 'unable to open database file' while the run still printed a well-formed summary and exited 0. Both seed_baseline() and worker.py's make_session_factory() now use the same absolute path. - Crashed workers' stderr was captured then discarded, and main() always exited 0 -- an all-crashed sweep was indistinguishable from a real (uneventful) measurement to anything checking the exit code or --out. run_workers() now prints each crash immediately and tags it with the real worker_id (previously always None); summarize() exposes crashed_worker_errors alongside the existing crashed_workers count; main() exits 1 via the new summary_indicates_failure() whenever any sweep crashed or fell short of expected_total_ops. - idx_users_oauth_identity was hardcoded separately in the ORM Index and in _is_oauth_identity_violation's Postgres branch, with no test to catch drift, and that branch had zero non-skipped coverage (its only guard needs a live Postgres CI never configures). Exported OAUTH_IDENTITY_INDEX_NAME from user/model.py as the shared source of truth (migrations intentionally keep their own frozen literal, matching every other revision in that package) and added stub-exception unit tests pinning both the asyncpg constraint_name path and the sqlite message-substring path, positive and negative. Verified: 107 passed locally (auth + bench-unit suites), ruff clean, and two real reproductions -- invoking run_concurrency_bench.py from a scratch directory outside backend/ (the reviewer's exact repro) now completes 8/8 ops with crashed_workers: 0 instead of crashing, and the new crashed_worker_errors/exit-code logic is exercised directly by the new unit tests against the real summarize()/summary_indicates_failure(). * fix(auth): attribute create_user IntegrityErrors to the right constraint Two coupled review findings on the classification helpers: P3 (fall-through) -- after ruling out the OAuth-identity index, create_user raised "Email already registered: {email}" for every remaining IntegrityError, including the duplicate-primary-key case the new regression test exercises, whose address is not registered. Added _is_email_violation() so the email message is used only for an actual users.email collision that raced past the pre-check; anything else (in practice a duplicate id) now raises a neutral "User already exists (constraint: <name>)". P2 (unreachable asyncpg branch) -- exc.orig is not the asyncpg error. SQLAlchemy's asyncpg dialect re-raises its own DBAPI IntegrityError (pgcode/sqlstate only) 'from' the real asyncpg error, so constraint_name lives on exc.orig.__cause__. getattr(exc.orig, "constraint_name", None) was always None on Postgres; the helpers only worked there by accident, matching asyncpg's DETAIL line in the message fallback. Added _driver_constraint_name() which walks orig then orig.__cause__, and the stub tests now model that real shape (orig wrapper + __cause__) instead of a constraint_name that no driver puts on orig directly. Tests: 76 passed. New coverage for the email-race path, both new helpers on each backend, the neutral fallback message, and the cause-chain walk. * fix(bench): match app SQLite PRAGMAs in workers; fail a sweep on any op error Two review follow-ups: - worker.py opened its SQLite engine with only connect_args timeout=30. synchronous and foreign_keys are per-connection PRAGMAs, so workers ran at SQLite's synchronous=FULL / foreign_keys=OFF while a real Gateway worker runs synchronous=NORMAL (persistence/engine.py::_enable_sqlite_wal) -- an extra fsync per commit on the measured 30%-write path, overstating SQLite's cost in the direction that flatters the "use Postgres" conclusion. Added a connect listener applying the same four PRAGMAs, with a test asserting synchronous/foreign_keys/journal_mode on a real worker connection. - summary_indicates_failure() only looked at crashes and the completed vs expected op counts, so a sweep where every op completed but raised (e.g. writes hitting OperationalError) passed as a clean measurement: completed_ops == expected, 0 crashes. Added an "errors > 0" clause; the error breakdown stays in the JSON, only the exit code changes. Test added. test_bench_concurrency.py + test_bench_worker.py green (20), plus a real 2-worker sqlite smoke run (6/6 ops, 0 errors, exit 0). * fix(auth): match the real email index name; only claim "exists" for uniqueness Review follow-ups on the classification helpers: - email is mapped_column(unique=True, index=True), which SQLAlchemy and 0001_baseline realise as a single UNIQUE INDEX (ix_users_email), not a named UNIQUE constraint. _is_email_violation compared the driver constraint name against "users_email_key", which Postgres never emits, so that arm was dead on Postgres (SQLite matched via the message). Fixed to ix_users_email. - the residual IntegrityError fallback raised "User already exists" for every remaining IntegrityError -- a NOT NULL / CHECK / foreign-key violation is not a "user already exists" condition and is not part of create_user's ValueError contract. Added _is_uniqueness_violation (sqlstate 23505, or the SQLite "UNIQUE/PRIMARY KEY constraint failed" message); only that raises the "already exists" ValueError, everything else propagates unchanged. - documented scripts/benchmark/concurrency/ in backend/AGENTS.md alongside the other benchmark family. Tests: 78 auth + 20 bench-unit pass, ruff clean. New coverage for _is_uniqueness_violation on both backends and for a non-uniqueness IntegrityError propagating out of create_user. * fix(bench): don't pre-close worker stdin (breaks communicate); require --pg-url for postgres * fix(bench): ruff format; time throughput on the op phase, not teardown - lint-backend: ruff format the files touched in this PR. - Throughput window (P2): the orchestrator sampled its wall clock after every worker's communicate() returned, so it also covered each worker's engine.dispose(), result serialization and stdout transfer. Each worker now times just its operation phase (GO -> last op) and reports ops_elapsed_s; summarize() uses max(ops_elapsed_s) over the workers -- all released by the same GO -- as the throughput window (ops_window_s). - Exercise migration 0018 (P2): test_user_oauth_partial_index.py goes through bootstrap create_all(), which builds the partial index from ORM metadata and never runs 0018.upgrade(). New Postgres-gated test_migration_0018_oauth_identity_pg_partial.py alembic-upgrades to 0017 (full index), then 0018 (asserts the predicate appears), then downgrades (asserts the full index is restored) and re-upgrades. * docs(middlewares): tighten SandboxAudit and Clarification entries in AGENTS.md PR #5134 grew agents/middlewares/AGENTS.md ~1.8 KB, pushing the effective AGENTS.md chain for that directory over the 96 KiB hard limit once this branch also documents scripts/benchmark/concurrency/ in backend/AGENTS.md. Condense the two longest middleware entries (SandboxAuditMiddleware, ClarificationMiddleware) without dropping any identifier, example, issue reference, ordering constraint, or documented gap; chain back to ~96.8 KiB. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-09-03 08:17:26 +08:00
Ishaan Potle	8ee3c83508	fix(browser): keep references to detached live-frame tasks (#5155) ... BrowserSession scheduled three coroutines with a bare asyncio.ensure_future(), so nothing held a reference to the resulting tasks. The event loop only keeps weak references, so such a task can be garbage collected before it finishes. For the two live-frame schedulers the consequence is worse than losing the task. Each sets a *_pending guard before scheduling and clears it in a finally block: self._settle_live_frames_pending = True asyncio.ensure_future(self._settle_live_frames()) If the task is collected, the finally never runs, the guard stays True forever, and every later _schedule_settle_live_frames()/ _schedule_input_live_frame() call returns early — silently stopping live frame refresh for that session with no error. Add _spawn_background(), which retains the task in a set and discards it on completion, and route the three call sites through it. This matches the pattern already used in task_tool, session_pool, notify and others. Add regression tests asserting the task is retained across a gc.collect() and released once it completes.	2026-09-03 08:04:21 +08:00
SPEC	822c7bca4b	fix(memory): cancel buffered extraction when agent is deleted or cleared (#5123) ... * fix(memory): cancel buffered extraction when agent is deleted or cleared * fix(memory): cancel buffered work before agent delete Address review: cancel before/after delete to close the rmtree race, scope user_id=None cancels to the legacy root only, and import memory helpers at module scope. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * fix(memory): close remaining cancel races from review Post-clear cancel, legacy-only all_agents scope, always cancel even when memory is disabled, and fold cancel+delete into one offloaded thread. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * docs(memory): align cancel_by_agent None-scope with legacy root Document that user_id=None cancels only the legacy no-user bucket, matching clear/storage semantics, not the whole process-local queue. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * test(memory): fix cancel_by_agent docstring regression assertion Signed-off-by: SPEC <zt1y17@soton.ac.uk> * fix(memory): address final cancel review nits Type the delete helper with AgentStore, replace docstring pinning with a kwargs mapping test, and document scoped cancel + residual window in AGENTS.md. Signed-off-by: SPEC <zt1y17@soton.ac.uk> * fix(memory): resolve agent store inside delete worker thread get_agent_store() does blocking config/FS work; keep it off the event loop so test_delete_agent_does_not_block_event_loop and backend-blocking-io CI pass. Signed-off-by: SPEC <zt1y17@soton.ac.uk> --------- Signed-off-by: SPEC <zt1y17@soton.ac.uk>	2026-09-03 08:00:25 +08:00
Willem Jiang	037658b0ee	fix(ci):reduce the size of AGENTS.md in sandbox (#5146)	2026-09-02 22:36:32 +08:00
gus	47f43f79f4	fix: improve local environment detection guidance (#5111) ... Co-authored-by: angus-guo <217034332+angus-guo@users.noreply.github.com>	2026-09-02 21:35:33 +08:00
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	2026-09-02 21:05:23 +08:00
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>	2026-09-02 19:16:56 +08:00
nicochow	bbcfd368bf	fix(sandbox): scrub SSH_AUTH_SOCK from the sandbox subprocess env (#5145) ... SSH_AUTH_SOCK points at the host's ssh-agent socket. A sandbox subprocess that inherits it can sign and authenticate with every key the agent holds (git push, ssh logins) without reading any key file -- the same credential-pointer leak class as the *_ASKPASS helpers the env policy already scrubs deliberately. No wildcard pattern fits (*AUTH* would strip benign names), so add an exact entry to _BLOCKED_EXACT_NAMES. A skill that genuinely needs the agent socket can still declare it via required-secrets: injected values win over the blocklist by design. Co-authored-by: zhouyujie <zhouyujie@keep.com>	2026-09-02 19:05:50 +08:00
qian	30788c79ff	fix(title): ignore upload context in conversation titles (#4729) ... * fix(title): ignore upload context in conversation titles * fix(title): cover attachment-only conversations * fix(title): skip model for attachment-only messages * fix(title): handle whitespace-only user content --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-09-02 14:40:12 +08:00
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.	2026-09-02 12:34:09 +08:00
AoHanBei	9b32b5d841	feat(observability): persist loop detection events (#5127) ... * feat(observability): persist loop detection events * fix(observability): persist subagent loop events * fix(observability): narrow subagent loop event bridge * fix(observability): attribute subagent loop events * fix(tests): isolate subagent executor imports	2026-09-02 10:25:37 +08:00
Jeremy Schoemaker	6b4f803354	fix(sandbox): preserve trailing whitespace in filenames from list_dir and glob in remote providers (#4980) ... * fix(sandbox): stop stripping filenames when parsing find output in remote providers The list_dir and glob parsers in the e2b, OpenSandbox, AIO, Tenki, and BoxLite providers called .strip() on every line of find output. A filename that legitimately ends (or begins) in whitespace was corrupted, so the listed path never resolved on any follow-up file API call, and the remote providers diverged from LocalSandbox, which preserves such names via pathlib. splitlines() already removes the line terminators, so filter empty lines only and keep each entry verbatim. Same class of bug as the e2b _sync_outputs_to_host fix (#4861), applied to the search parsers. Adds a trailing-space regression test per provider at the seam each suite already uses. * fix(sandbox): split find output on \n only, and rename the tenki test Review follow-ups from willem-bd: - aio_sandbox.list_dir used str.splitlines(), which also breaks records on \v, \f, \x1c-\x1e and \x85 - all legal inside a Linux filename, and all contrary to this PR's own rule that the newline is the only delimiter. find emits \n and nothing else, so split("\n") is the correct parse. - Renamed test_search_preserves_trailing_space_in_filename to test_list_dir_and_glob_preserve_trailing_space_in_filename, matching the sibling tests in test_opensandbox_provider.py and test_boxlite_provider.py. The body covers list_dir and glob; it never touches grep.	2026-09-02 09:23:04 +08:00
Madan kumar	df68b59149	fix(skills): reject a blank SKILL.md description at the write gate (#4867) ... * fix(skills): reject a blank SKILL.md description at the write gate _validate_skill_frontmatter only applied its description rules when the value was truthy, so a blank or whitespace-only description passed the gate. The loader rejects it, so the file was written to disk and then disappeared from every consumer. On the PUT edit endpoint that meant the write committed first and the response was a 404 -- the previously working skill was gone with no rollback. Same shape via the .skill install path and the agent-facing skill_manage tool, which reported success for a skill that never loads. Empty names were already rejected; description was the only field where the write gate and the loader disagreed. * test(skills): pin rollback rejection of a blank-description history entry Add a regression test for the rollback path: restoring a history entry whose stored content has an empty description must return 400 ("Description cannot be empty") and leave the on-disk SKILL.md untouched, rather than the previous destructive path that wrote the unloadable content and then 404'd. On main this returns 404, so the test also pins the intended status-code change on this path.	2026-09-02 00:33:21 +08:00
Ricky-7-Yan	c56c7293f8	test(checkpoint): measure postgres storage growth (#5051)	2026-09-01 22:36:31 +08:00
rayhpeng	cd35363a05	fix(history): early user messages vanish or jump mid-run when pagination and context compaction overlap (#4696) ... * fix(history): stop dropping user messages that fall outside the loaded page window Two independent paths made a user's own message disappear from a long thread (#4666, #4508, #4363). Both are reproduced by a real two-round run: once the thread passes the 50-row `/messages/page` window AND context compaction fires, the two sources of truth stop overlapping at the head. 1. Middleware-answered tool results never reached the event store. A middleware that short-circuits a tool call (e.g. ReadBeforeWriteMiddleware's blocked write) returns a user-visible ToolMessage, but LangChain never emits `on_tool_end`, so RunJournal never persisted it — the user saw it during the run and it vanished on reload. RunJournal already reconciles final-output tool messages, but only for an `ask_clarification` allowlist. The allowlist is removed; scope stays bounded by the three conditions that actually matter (visible, this run's lead agent, not already persisted), so subagent results still stay in their own step feed. 2. mergeMessages discarded the checkpoint prefix before the first shared anchor. #4065 correctly established that a summarization-rescued early message must not be appended to the tail, and suppressed it instead. That suppression is what deletes the message when the first history page no longer reaches back to it. It is now woven in before the first shared anchor — the one position both the checkpoint and seq-sorted history agree on — so #4065's invariant (never the tail) still holds. A collapsed unloaded gap is recoverable by paging; a dropped message is not. Verified against real captured payloads from the reproducing run: the first user message returns to the transcript. Its exact position is still approximate — after compaction the live window carries too few anchors to place it precisely, which only seq-based ordering can close. Backend: 10809 passed (baseline 10808; same 15 pre-existing failures in browser/crawler community tools). Frontend: 986 passed, typecheck + eslint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(events): look up a persisted message's seq by identity Groundwork for placing checkpoint messages in the seq-ordered thread feed (#4666). A checkpoint carries no seq of its own and loses messages to summarization, so once the feed's 50-row page window no longer reaches back to a surviving old message, a client has nothing to place it by. The seq already exists in run_events keyed by the message id — this exposes it without paging the whole feed. `message_identity` is the backend half of the identity rule the frontend applies in `hooks.ts::messageIdentity`: a ToolMessage is keyed by `tool_call_id`, and DynamicContextMiddleware's `X` / `X__user` human copies collapse to one identity. The two halves must stay in sync — a mismatch is silent, degrading placement rather than raising. `get_message_seqs` is implemented for all three stores. Misses are absent from the result rather than an error, so callers degrade to their own placement rule; the earliest seq wins when one identity resolves to several rows, so a re-persisted message keeps the position it first occupied. The DB store decodes rows in Python because `content` is a TEXT column holding a JSON string, not a JSON column — the identity fields cannot be projected in SQL. Nothing consumes this yet; no behavior change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(runtime): carry each persisted message's feed seq on values frames Attaches `additional_kwargs.deerflow_seq` to messages in a root `values` frame that the thread feed already holds, so a client can place a message the checkpoint kept but its loaded history page window no longer reaches (#4666). Nothing is written back to the checkpoint: the seq is added when the frame is serialized and belongs to that frame only. Cost is bounded to frames introducing identities the run has not resolved yet. Messages this run produces are not in the feed while streaming, so they are looked up once, recorded as misses, and never retried — in a real run the only frame that pays for a query is the one where compaction brings older messages back into view. Measured on a reproducing two-round run: 1 lookup across 25 values frames. The stamper is built once per run rather than per `_stream_once`, or a goal continuation would discard the resolved seqs. Subgraph frames are not stamped: a subagent's snapshot is not part of this thread's feed ordering. A lookup failure logs and leaves the frame unstamped rather than failing it — placement is an enhancement and clients fall back to their own ordering rule. Frontend does not read the field yet; no behavior change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gateway): strip the server-owned message seq from untrusted input `deerflow_seq` is display metadata the Gateway attaches when it serializes a values frame. A client replaying messages (regenerate / edit-and-rerun) would otherwise write it into the checkpoint, where it becomes wrong the moment the thread is forked — a branch re-seeds its feed and reassigns seq (#4380). Joins the existing server-owned key set, so it follows the same trusted-internal rule as the dynamic-context and view-image markers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): place a checkpoint message by its feed seq, not its nearest anchor Completes #4666. Weaving a compaction-rescued message before the first shared anchor keeps it in the transcript, but not in the right place: after compaction the live window carries too few anchors, and the nearest one can sit deep inside the loaded page window — measured at row 25 of 50 on a reproducing run, which is why the first user turn rendered mid-transcript instead of at the head. Both sides now carry the backend's thread-global seq. `buildVisibleHistoryMessages` copies each row's `seq` onto the message (same shape as the existing `run_id`), and the Gateway stamps it onto `values` frame messages it has already persisted. A live message whose seq is below the loaded window's lower bound is placed ahead of everything on screen rather than before the nearest anchor. A message with no seq — still streaming, so not in the feed yet — keeps the weaving path, since the tail is already its correct position. Verified against the captured payloads of the reproducing run: the first user message goes from absent, to #13 (behind the second question), to #0. Frontend: 988 passed, typecheck + eslint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): place a pre-window checkpoint message even when no anchor is shared Also #4666. Placing a compaction-rescued message by its feed seq was gated on reaching a shared anchor, because the split ran inside the anchor walk. When the loaded page and the live checkpoint share no identity at all, that walk never runs and the message fell through to `[...canonical, ...live]` — appended after the entire window, the one arrangement #4065 proved wrong, with its seq known the whole time. That is not a corner case. Open an old, already-summarized conversation and send a message: the page on screen is the newest rows from before that turn, while the checkpoint holds the rescued first user turn plus steps of the new run that are not in the feed yet. On a reproducing run the two sides shared zero anchors and the user's own first question rendered at row 50 of 50 — the reported "first message jumps to the bottom". Split `beforeWindow` out of `live` before walking anchors, walk `liveInWindow`, and use it for the no-anchor branch as well, so a message routed ahead of the window is not re-appended at the tail by dedup. Measured on captured payloads of a reproducing run (real gateway, real compaction), first user message position: no shared anchor: row 50 -> row 0, seq order monotonic again shared anchors: row 0 -> row 0 (unchanged) paged to the top: row 0 -> row 0 (unchanged) Regression test verified red-green: reverting the fix fails it with the message rendered after the window. Frontend: 989 passed, eslint + tsc clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gateway): stamp the message feed seq on checkpoint reads, not only on stream frames Completes #4666. `_MessageSeqStamper` sits on the streaming publish path, so a client that joins a live run learns where a summarization-rescued turn belongs while a client that merely opens the conversation does not — and opening is the common case. `GET /threads/{id}/state` and `POST /threads/{id}/history` returned the checkpoint with no seq at all, so the merge fell back to the nearest shared anchor, which after summarization sits deep inside the loaded page. Reproduced in a browser against a real gateway, on a thread that had already compacted: the user's first question rendered at row 320 of 389, behind the newest question instead of at the head. Both reads showed 0 of 13 messages carrying a seq. That is the reported symptom, still present after the streaming fix. Add `stamp_messages_with_seq`, the request-scoped counterpart of the stamper: everything a checkpoint still holds is already persisted, so one batched lookup resolves the whole list and there is nothing to retry later. Resolve the store through `_optional_run_event_store` rather than `get_run_event_store`, because seq is placement metadata — a deployment without a feed must still be able to read a thread. After the fix, on the same thread in the same browser: 13 of 13 messages carry a seq and the first question renders at the head, ahead of the newest one. Backend: ruff clean, 326 passed across the touched suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(harness): move the injected-user-id suffix helpers to utils.messages to break an import cycle message_identity imported strip_injected_user_message_id_suffix from the dynamic-context middleware, closing a cycle (middleware -> deerflow.runtime -> worker -> events -> middleware) that only stayed hidden while an earlier import happened to break it. Define INJECTED_USER_MESSAGE_ID_SUFFIX and the strip helper in deerflow.utils.messages and re-export them from the middleware so existing importers keep working. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(docs): improve formatting and clarity in AGENTS.md and message-merge.test.ts * perf(events): stop the seq scan once every wanted identity is resolved Rows past the last wanted seq can only be re-persisted copies that already lose the earliest-seq-wins tiebreak, so all three stores now break out of the scan (and the db store out of its per-row JSON decoding) once found covers wanted. Matters most for /state and /history reads of long threads, where this lookup runs with no run cache and a typically tiny wanted set. Raised by review on #4696. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(events): share the seq-stamping expression between the two stampers The walrus-plus-merge expression was duplicated verbatim between stamp_messages_with_seq and _MessageSeqStamper.stamp — two counterparts of one rule where silent divergence is the likely failure mode if only one side is edited. Both now call attach_message_seq next to MESSAGE_SEQ_KEY in message_identity.py. The trailing isinstance(message, Mapping) guard was unreachable (a non-Mapping entry already got identity = None) and is gone with the extraction. Raised by review on #4696. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(events): seq stamping survives launch paths without user context The db store's get_message_seqs defaults to user_id=AUTO, which raises when no user is in the contextvar — the first strict-AUTO read ever called from the worker context. On a launch path that never inherits the auth context (e.g. a null-owner scheduled task), stamp()'s except clause swallowed that into a per-frame warning and silently disabled seq stamping for exactly the background runs that need it. The stamper now soft-resolves the user id once at build time — the same rule as the worker's write paths beside it (unset -> no filter) — and passes it explicitly. jsonl/memory stores gain the same user_id kwarg the base list_messages contract already carries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(events): SQL-prefilter the message seq lookup's candidate rows get_message_seqs scanned and JSON-decoded every message row of the thread: the early exit never fires when a wanted identity is absent from the feed (a message still streaming, or checkpoint-only), and /state / /history reads want the newest messages, so the ascending scan traversed essentially the whole feed — with the content column carrying full tool outputs, that is heavy I/O plus N JSON parses on exactly the long threads this lookup exists for. A LIKE prefilter now keeps that cost in SQL: only rows containing a wanted raw id as a substring are fetched and decoded. False positives are re-checked by message_identity; LIKE wildcards are escaped; an id json.dumps would escape (breaking the verbatim-substring guarantee) falls the whole set back to the full scan rather than silently missing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(agents): sink runtime mechanism docs below the gateway guidance budget Merging main pushed backend/app/gateway/AGENTS.md past its 40KB soft budget (main had left 81 bytes of headroom). Per the nearest-file rule, move the mechanism detail of the message-seq stamping and run-delivery receipt sections — both owned by runtime/ code — into packages/harness/deerflow/runtime/AGENTS.md, leaving the gateway file the REST-surface summary and a pointer. The seq section also documents the stamper's build-time soft user-id resolution and the db store's SQL prefilter from the review follow-ups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(agents): sink durable-MCP task detail below the backend guidance budget Merging main pushed backend/AGENTS.md past its 24KB module soft budget (main itself is at 24762 after #4848 — this branch adds zero net bytes to the file). Per the nearest-file rule, move the two durable-MCP task runtime bullets' mechanism detail into packages/harness/deerflow/mcp/AGENTS.md, leaving summaries and pointers; this also restores ~2KB of headroom so the next merge does not trip the same wire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(events): re-ask a message-seq miss once the feed advances The run-scoped stamper cached lookup misses for the whole run. A message this run produces reaches a values frame before RunJournal flushes it, so its first lookup legitimately misses — and the journal persists it moments later, giving it a feed seq the stamper never asks for again. A long run that afterwards rolls past the history page and compacts then carries that message unstamped, back to the approximate anchor placement this stamper exists to replace (#4666). A transient store error had the same permanent effect, since the except clause degrades to an empty result. A miss is now provisional while a hit stays final: RunJournal counts its successful event-store writes as `feed_generation`, and the stamper re-asks a missed identity only once that counter moves. Retrying is therefore bounded by feed writes rather than by frames — the per-frame query the run-scoped cache was built to avoid — and a failed lookup costs one generation instead of the run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-01 22:04:17 +08:00
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	2026-09-01 22:01:21 +08:00
Hyeonsang Cho	9146bfa03d	feature(gateway): issue request trace ids unconditionally (#5119) ... * refactor(gateway): issue request trace ids unconditionally The request trace id was gated behind logging.enhance.enabled at every entry point, so downstream code had to keep asking whether one existed: a header-provenance flag in its own ContextVar, a precedence resolver, and three-level carrier fallbacks at each consumer. Bind one unconditionally instead. TraceMiddleware covers Gateway HTTP; ensure_trace_context covers the entry points that never touch ASGI -- scheduled occurrences, MCP task notification runs, IM channel messages, and the embedded client -- each scoped to one unit of work so a long-lived worker task cannot leak one occurrence's id into the next. The ContextVar becomes the only source; the response header, runtime context, run metadata and log records are derived outputs. Consumers now use ensure_trace_id() or resolve_trace_id(*carriers) and drop their presence guards. Removed: resolve_deerflow_trace_id, the header-provenance flag and its three helpers, set/reset_current_trace_id, is_trace_correlation_enabled and its gateway alias. BREAKING CHANGE: every Gateway HTTP response now carries X-Trace-Id and it cannot be turned off; logging.enhance.enabled controls log output only. Installations on the default enabled: false will start seeing the header. No config keys were added or removed. * fix(gateway): stop persisting a caller-supplied trace id on the run record body.metadata forks two ways: through build_run_config into the live run config, which the run worker restamps, and through create_or_reject into the run record that the runs API echoes verbatim. Only the first was covered, so a client sending metadata.deerflow_trace_id made the most durable and most visible surface of a run disagree with the X-Trace-Id and the log lines the same request produced -- a correlation id that does not match the logs is worse than none. Stamp the server-issued id once at the trust boundary so both forks receive it, preserving the caller's own metadata keys. Close the same gap on config.context, which reaches the runtime context by a separate path: _build_runtime_context no longer merges server-owned keys from the caller, and _install_runtime_context assigns rather than setdefaults. A thread's metadata is no longer seeded with the run-scoped id of whichever run created it -- one thread spans many runs and as many trace ids. Found by driving a real run through the Gateway and reading the run back from the runs API; every unit test built its metadata by hand and so could not see it. * fix(gateway): expose X-Trace-Id to split-origin browser clients X-Trace-Id is not on the CORS safelist, so a browser client served from a separate origin could not read it -- and those are exactly the clients that cannot read the Gateway's logs either, leaving them with nothing to quote in a bug report. Same-origin nginx deployments were unaffected, which is why this stayed hidden. Add it to CORS_EXPOSED_HEADERS beside Content-Location, referencing TRACE_ID_HEADER rather than repeating the literal. * fix(gateway): keep X-Trace-Id on unhandled-exception 500s Starlette's ServerErrorMiddleware sits outside every user middleware and emits unhandled-exception 500s through the raw send, so those responses never pass TraceMiddleware's header-writing wrapper. The 500 for a server bug is exactly the response a user most needs to correlate with a log line, and it was the one response that shipped without the id. TraceMiddleware now tracks whether http.response.start has been sent. On an exception with no response started it emits its own plain 500 carrying the header, then re-raises: the outer ServerErrorMiddleware sees the response already started and only re-raises too, so the server's exception logging is untouched. An exception mid-stream keeps propagating unchanged — a second response start cannot be sent, and the already-written header stands. The trace id is printable ASCII by construction (normalize_trace_id / generate_trace_id), which is what makes the raw latin-1 header encoding safe. * fix(gateway): strip the forged trace id from the persisted request echo The run-record fix stopped a forged metadata.deerflow_trace_id on the authoritative metadata surface, but the raw request echo still carried one: create_or_reject persists body.config verbatim as runs.kwargs_json, which the runs API serves back. A client posting config.context.deerflow_trace_id therefore still got its forged value stored and echoed on one API surface while the header, logs, run metadata, and checkpoint all carried the real id — the id is ignored as input there, so echoing it back only manufactures disagreement. Two changes close it. redact_config_secrets — already the shared scrub for that echo, applied at admission and again at serve time, so historical records are covered too — now also drops deerflow_trace_id from config.metadata and config.context. And build_run_config now merges run metadata onto a copy of the caller's config["metadata"] instead of updating it in place: the nested values of the request config are reference copies, so the in-place merge was writing the server-stamped key through into body.config, contaminating the "what the client sent" record before it was persisted (and incidentally masking the forged-value echo on the metadata container). The regression test posts a forged id through body.metadata, config.metadata, and config.context at once and reads the kwargs echo back off the run record, failing if either leak returns. * docs(harness): record the trace-echo scrub, 500 fallback, and accepted retry divergence The trace section of the harness AGENTS.md now covers the two fixes that close the derived-output rule (the kwargs-echo scrub in redact_config_secrets plus build_run_config's copy merge, and TraceMiddleware's own 500 for unhandled exceptions), and CHANGELOG gains their Fixed entries. It also writes down the one accepted divergence: a crash-recovered scheduled launch reuses the durable run through its idempotency key, and start_run returns early on idempotency_reused without restamping — so the run record keeps the first attempt's deerflow_trace_id while the retry's own log lines carry the freshly minted id of its ensure_trace_context binding. The divergence is confined to the crash-recovery window and is accepted rather than fixed: restamping on reuse would rewrite a persisted record for a run that already exists, which is worse than two ids that each correlate their own attempt's logs. Written down so the next reader of the scheduler recovery path does not diagnose it as a bug. * docs(config): align the logging.enhance schema note with the unconditional trace id The config-module AGENTS.md still described logging.enhance as the gate for the Gateway X-Trace-Id header and Langfuse deerflow_trace_id. That model is gone: ids are issued unconditionally and this block decides log output only. Left as-is, the stale wording invites an agent to "restore" a header gate it believes was lost. Reworded to match the sibling AGENTS.md files and config.example.yaml, with a pointer to the Request Trace Context section that owns the full model. * docs(changelog): link the trace entries to #5119 The five new entries pointed at the ([#XXXX]) placeholder with no reference definition, rendering as literal text instead of a link — and RELEASING.md step 2 relies on those references when the section becomes release notes. All five now point at #5119, with the definition appended to the reference block. * refactor(harness): rename _stream_without_trace_context to _stream_turn The name asserted the opposite of what the method now does. It was accurate while logging.enhance.enabled could route stream() around the trace scope; with the gate gone it is the only stream implementation left, and it binds the id itself via ensure_trace_id(). Private, so the rename touches only the definition and the one stream() call site. * docs(harness): fit the trace-context guidance inside the AGENTS.md chain budget The expanded Request Trace Context section pushed the effective AGENTS.md chain for agents/middlewares to 99,815 bytes, past the 98,304 hard limit scripts/check_agent_guidance.py enforces in CI (AG002). Compressed the section from 7,359 to 4592 bytes with no facts removed: the entry-point table, the derived-output rule and its enforcement points, the accepted scheduled-retry divergence, the two resolution helpers, the stream() binding rationale, the log-output-only gate, the CORS listing, the 500 fallback, and the test map all remain. Sized against the merge, not just the branch: current main grew the same chain by ~724 bytes, so the check was verified on the merged tree as well (97,772 bytes; branch tree 97,048). * fix(gateway): declare content-length on the fallback 500 The pre-response 500 declared content-type but no content-length, leaving the framing to the ASGI server: chunked on HTTP/1.1, close-delimited on HTTP/1.0 — the one wire difference from the ServerErrorMiddleware response it replaces, which sends content-length: 21. The explicit header keeps the fallback byte-identical to what clients saw before. * docs(readme): drop the trace-correlation condition from the translations The zh/ja/fr/ru Langfuse sections still said metadata.deerflow_trace_id matches X-Trace-Id "when request trace correlation is enabled". The id now always matches and that condition no longer exists, so each bullet states the unconditional match and that logging.enhance.enabled only controls whether the id is printed into logs — the one piece of the feature a user can still configure. * test(gateway): pin TraceMiddleware wiring through create_app() Every X-Trace-Id test exercised a hand-built four-route app, so the real stack's add_middleware(TraceMiddleware) line was pinned by nothing: deleting it — or short-circuiting above it — passed CI while silently dropping both the response header and the ambient id the run-record stamp and enhanced log records derive from. One case now drives /health through create_app() and asserts the inbound id round-trips; mutation-checked by removing the wiring line, which fails exactly this test. * docs(gateway): note the fallback 500 is CORS-opaque The pre-response 500 is emitted outside CORSMiddleware — the exception has already unwound past it — so it carries no Access-Control-Allow-Origin and a split-origin browser client cannot read the id on this one response, unchanged from the ServerErrorMiddleware 500 it replaces. Documented on the class and in the CHANGELOG entry rather than fixed: replicating the origin allowlist outside CORSMiddleware would let the two policies drift. * fix(harness): keep abandoned-stream cleanup inside the trace binding stream() binds the turn's id around each next(inner) and resets it before yielding, but the finally's inner.close() ran after that binding was gone. Abandoning the stream therefore drove the inner LangGraph generator's GeneratorExit/finally path with no trace id — or an unrelated ambient one from whichever context ran the close — so cancellation and finalization logs and callbacks did not correlate with the turn they belong to. inner.close() is now wrapped in a local bind/reset of the same turn id. The token is set and reset in the same frame, never across a yield, so the per-step cross-context safety is preserved even when GC closes the generator from another Context — pinned by the existing copy_context close test, which now exercises this path. The regression test records the id from the inner generator's finally and fails without the binding. * test(harness): teach the worker-trace fake about RunManager.cleanup Upstream #5112 (bound gateway memory after terminal runs) added a run_manager.cleanup(run_id) call to run_agent's finalization, so the merge-commit CI run failed all five worker-trace-binding tests with AttributeError on this PR's _FakeRunManager. The fake gains the same no-op shape as its other methods. * docs(gateway): bring the gateway AGENTS.md back under its soft budget Upstream #5092 grew backend/app/gateway/AGENTS.md to 40,966 bytes, 6 over the 40,960 soft budget that test_agent_guidance_check.py::test_repository_guidance_stays_below_soft_budgets_and_avoids_doc_indexes enforces — its Unit Tests run on main was cancelled by push concurrency, so main is currently red on that test and every PR merge-run inherits the failure. Two whitespace/wording trims in the row #5092 touched (a doubled space, and "its configured `context_window`" → "its `context_window`") bring the file to 40,953 with no content change. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-09-01 16:49:39 +08:00
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)	2026-09-01 16:13:41 +08:00
Daoyuan Li	df57f8e269	fix(lark): preserve app secrets during credential switches (#4820)	2026-09-01 15:37:19 +08:00
betterkite	b552b5015c	fix(messages): drop legacy <uploaded_files> tag handling (#4826) ... * fix(messages): drop legacy <uploaded_files> tag handling (#4212) PR #4174 unified upload-context injection on <current_uploads> (IM and web both flow through UploadsMiddleware), and #4632 documented the current path. This removes the remaining backward-compat parsing of the pre-#4174 <uploaded_files> tag, the final cleanup item tracked by the issue: - deermem: only <current_uploads> is stripped from human turns before memory persistence, and the upload-sentence scrubber drops the legacy tag alternative. - mem0: the mirrored message filter recognises only <current_uploads>. - InputSanitizationMiddleware: remove the legacy tag from the blocked-tag denylist (it existed only because deermem parsed the old tag). - frontend: stripUploadedFilesTag / stripInternalMarkers / parseUploadedFiles and the message-list fallback parse only <current_uploads>; demo thread fixtures are migrated to the current tag. Scope decision: a <uploaded_files> block in pre-#4174 history is now treated as ordinary user content (pinned by tests in both layers) instead of being silently dropped or stripped. * style: apply prettier formatting to stripUploadedFilesTag * fix(uploads): keep legacy <uploaded_files> stripping for display/export only Addresses review feedback on #4826: removing the legacy tag from the frontend display layer made pre-#4174 threads render raw <uploaded_files> XML (with server-side upload paths) in chat, copy data, and JSON exports. The backend cleanup stands — memory pipelines and the sanitization denylist treat only <current_uploads> as an internal marker. The frontend keeps the legacy spelling in its display/export-only utilities (stripUploadedFilesTag / INTERNAL_MARKER_TAGS / parseUploadedFiles and the message-list fallback) so old history renders cleanly without leaking internal paths, while the memory/sanitization scope-decision tests remain unchanged. Frontend tests now pin both spellings: <current_uploads> and legacy <uploaded_files> are stripped from copy data, markdown leak-stripping, and JSON exports. * docs(ui): record accepted display-spoof tradeoff for legacy upload tag Review note (willem-bd): since <uploaded_files> is off the sanitization denylist, a live user can type the legacy spelling and fabricate file chips / hide their own message text in display. Display-only and self-inflicted with no backend semantics, so it is accepted for now; documented at both the message-list fallback and stripUploadedFilesTag. Age-gating the legacy spelling remains a possible follow-up. --------- Co-authored-by: betterkite <313258397+betterkite@users.noreply.github.com>	2026-09-01 15:26:33 +08:00
Aari	cdc886ae85	fix(sandbox): make tool descriptions optional (#4878) ... * fix(sandbox): make tool descriptions optional * fix(sandbox): address optional description review * test(sandbox): pin optional description contracts	2026-09-01 14:15:26 +08:00
Janlay	45adb8fbb5	perf(runtime): bound gateway memory after terminal runs (#5112) ... * fix(runtime): clean up terminal run records * perf(sandbox): bound local path caches * perf(runtime): release terminal run cycles * fix(runtime): address terminal cleanup review * fix(runtime): clean up after end publish failure * fix(runtime): guard terminal cleanup from cancellation * fix(runtime): discard fenced journal buffers * fix(runtime): harden abort and teardown paths	2026-09-01 10:56:43 +08:00
Yufeng He	a4f6665ef4	fix(security): sanitize MCP-sourced tool results through the same trust boundary (#4839) ... * fix(security): sanitize MCP-sourced tool results through the same trust boundary Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(security): sync the trust-boundary docs with tag coverage and pin the untagged branch Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>	2026-09-01 09:49:32 +08:00
hataa	17cac3420a	fix(subagents): clean up background task entry on unexpected poller exit (#5069) ... * fix(subagents): clean up background task entry on unexpected poller exit * fix(subagents): pin deferred cleanup to the persistent subagent loop The non-terminal fallback scheduled the deferred registry cleanup with asyncio.create_task on the poller's own loop. Under synchronous tool invocation the sync wrapper runs the tool coroutine through asyncio.run(), which cancels caller-loop tasks at teardown, so the cleanup died before executing and the _background_tasks entry leaked — the same lifecycle leak the terminal path already fixed. Schedule the deferred cleaner on the process-owned persistent subagent loop instead, via the new public executor helper run_on_isolated_subagent_loop (asyncio.run_coroutine_threadsafe). The cleaner only touches thread-safe registry helpers, so it is loop-agnostic. A caller-loop fallback remains for the unreachable case where the persistent loop cannot be obtained, so scheduling never raises out of an unwind path that is already handling an error. The polling-timeout return path, which shares the scheduler, is fixed the same way. Tests no longer stub the scheduler: the non-terminal fallback test drives the real scheduling wrapper on an equivalent long-lived loop and asserts cleanup runs after asyncio.run() tears the caller loop down, and run_on_isolated_subagent_loop itself is covered against the real persistent loop with the caller loop closed underneath. * fix(subagents): harden interrupted finalization against failing status path Three edge cases from review on the unexpected-exit unwind: 1. Finalization no longer depends on the failing status accessor. _peek_subagent_result distinguishes a gone entry from an unreadable one instead of letting the accessor's exception abort the unwind; _finalize_interrupted_subagent never raises (so the original poller exception is preserved) and attaches the deferred cleaner, whose last resort force-removes a persistently unreadable entry via the new executor force_cleanup_background_task. 2. The generic-error unwind waits only a short grace period (_UNEXPECTED_EXIT_GRACE_SECONDS) instead of the full execution timeout before re-raising; the remaining lifecycle stays with the deferred cleaner on the persistent subagent loop. 3. The deferred cleaner reports the subagent's final usage (deltas since the unwind snapshot included, via final=True bypassing usage_reported; the journal dedupes by source_run_id) before removing the terminal entry. The report is transferred in a plain worker thread so the RunJournal's loop-bound progress flush is skipped rather than scheduled on a foreign loop. * fix(subagents): pin deferred final usage delivery to the parent run loop _report_deferred_final_usage ran record_external_llm_usage_records in a to_thread worker, making it the first cross-thread RunJournal writer: the unlocked accumulators can lose token updates and _tokens_by_model mutations race get_completion_data() iteration on the parent loop. Capture the parent loop at unwind time (it is alive in every path that continues the run) and deliver the final report onto it with call_soon_threadsafe, serialized with all other journal access; when that loop is already closed (asyncio.run teardown) the report is dropped on purpose — the run has persisted and nothing reads the counters back. The live-loop test exercises the real recorder path (journal captures the running loop of every call) instead of stubbing _report_subagent_usage, so a cross-thread report would surface as a wrong-loop entry. Also gates the two teardown tests on the caller loop actually closing (the deferred cleaner could otherwise legitimately deliver while that loop is still winding down), and adds a task_tool-level regression test for execute_async submit failure leaving no registry residue (rolled back inside execute_async since #5086). * docs(subagents): document the reverse loop boundary; observability + test fixes Extend subagents/AGENTS.md's Isolated-loop callback boundary with the reverse-direction contract from #5069: deferred registry cleanup is pinned to the persistent subagent loop via run_on_isolated_subagent_loop, and the final usage report is handed back onto the parent run's loop captured at unwind time — never invoked from the persistent loop or a worker thread, which would silently reintroduce the journal accumulator/iteration race. Dropping the final report (closed parent loop) is the one path where a subagent's tail usage goes permanently unaccounted, so both drop branches now log at info with the execution id and the unaccounted record count. Restore the retention assert in test_deferred_cleanup_task_retained_and_ survives_gc: the bounded wait observes the production done-callback discard, the assert (not a manual discard) is what fails if that callback is deleted. * fix(subagents): honour grace-wait cancellation and shrink deferred-cleaner captures Two review follow-ups on the unwind: - The shared unwind absorbs CancelledError (never-raise contract), so a graph-node cancellation landing inside the generic-error grace wait was swallowed and the node ended as a failed tool call instead of an interrupted run. The generic branch now re-checks task.cancelling() after the unwind and re-raises CancelledError; the absorb site documents why the cancellation path needs it and where the discrimination lives. - The deferred cleaner captured the whole run runtime, pinning the parent run's journal and event store for up to a full poll budget (~31 min) via the strongly-held task handle — worst on the polling-timeout path, where a stuck subagent pinned its run's journal for a second full timeout after the tool returned. The recorder is now resolved on the unwind path and is the only capture (plus ids and the report loop); a None recorder skips reporting entirely. Both behaviours are regression-tested (red on the previous head, green after): a cancellation parked inside the grace wait surfaces as CancelledError, and the runtime is collectable while the cleaner still polls. The closed-loop drop test now uses a real recorder via runtime.callbacks so the drop it pins is loop-based, not recorder-absence.	2026-08-31 23:38:12 +08:00
Terminator666666	c17aa8b98f	fix(mcp): reject credentials that cannot travel as HTTP header values (#5066) ... * fix(mcp): reject credentials that cannot travel as HTTP header values A request-scoped secret or user_auth credential with a trailing newline (the usual result of reading a token from a file, or a CRLF env-file), CR/LF, surrounding whitespace, or characters outside Latin-1 sailed through the credential interceptors into the HTTP client, where httpx/h11 reject it with an exception that echoes the full value: LocalProtocolError: Illegal header value b'Bearer sk-...\n' ToolErrorHandlingMiddleware copies that message into a model-visible ToolMessage, so the secret landed in the prompt, the checkpoint, and traces - everywhere headers_from_context promises it never goes. Add illegal_header_value_reason to mcp/headers.py, mirroring the transport's own rules (Latin-1 encodable; h11's field_vchar is [^\x00\s] with SP/HTAB legal only between visible characters), and fail closed in both interceptors before the value can reach the client. The denial names only the secret key (plus the reason) and never repeats the value. Illegal values are denied regardless of on_missing: the key is present, so a passthrough fallback would silently run the call under the shared discovery credential - the exact authority confusion the deny default exists to prevent. Values the transport accepts are not rejected: embedded SP/HTAB ('Bearer <token>'), Latin-1 high bytes, and DEL all still pass, pinned by tests against h11's observed behaviour. * fix(mcp): tighten header value validation to httpx's ASCII boundary The validator mirrored h11's Latin-1 boundary, but the transport rejects more than h11 does: build_server_params hands dict[str, str] headers through the MCP SDK's create_mcp_http_client into httpx.AsyncClient, and httpx (pinned 0.28.1) encodes str header values as ASCII - so a Latin-1 high byte like 'Bearer caf\xe9' passed validation here only to raise UnicodeEncodeError inside httpx before h11 ever ran, with the exception message repeating the offending value. Validate str values against ASCII instead, flip the tests that pinned Latin-1 high bytes as transportable, and pin the boundary against the real client: create_mcp_http_client must reject what the validator flags and construct cleanly for what it accepts (embedded SP/HTAB and DEL still pass). Addresses review feedback on the ASCII vs Latin-1 boundary. * fix(mcp): validate OAuth and static header values at the same boundary The validator added for headers_from_context and user_auth left two paths uncovered. A token endpoint returning an access_token or token_type with a newline reached httpx/h11, which raise with the full token in the message, and ToolErrorHandlingMiddleware copies that message into a model-visible ToolMessage -- the leak this PR set out to close. The operator's static headers had the same hole. OAuthTokenManager.get_authorization_header now renders the Authorization value through one checked helper, so the tool interceptor, the initial discovery headers and the durable task path are all covered by a single guard. The rendered value is what gets checked rather than the two fields separately, because that is what the transport sees: an access_token with leading whitespace is legal once it follows "Bearer ". build_server_params applies the same check to statically configured headers. build_servers_config already isolates a per-server failure, so a bad value drops that one server and logs the reason instead of the value. * docs(mcp): correct which transport echoes the full header value The rationale claimed httpx and h11 both render the full value into their exception message. Only h11 does, on the line break and surrounding whitespace cases. httpx's ASCII failure is a UnicodeEncodeError naming the offending character and its position, not the credential, so at most one character escapes there; refusing the value up front buys an actionable error rather than an encode failure raised from inside the client. Corrected in headers.py and in every copy of the claim: context_headers.py, user_scoped_auth.py, oauth.py, client.py, mcp/AGENTS.md, docs/MCP_SERVER.md, the frontend mcp.mdx, and the test comments carrying the same wording. No behavior change. --------- Co-authored-by: Terminator666666 <Terminator666666@users.noreply.github.com>	2026-08-31 15:07:30 +08:00
Aari	317577e285	fix: enforce custom agent skill allowlists in sandboxes (#5077) ... * fix: enforce agent skill allowlists in sandboxes * fix: guard E2B skill projection resets * fix: preserve agent skill isolation across delegation * fix: close sandbox skill isolation bypasses * fix(sandbox): close skill isolation review gaps * fix(sandbox): harden skill isolation lifecycle	2026-08-31 14:35:08 +08:00
Ryker_Feng	8c8c5ac246	feat(search): add native recency filters (#5099) ... * feat(search): add native recency filters * fix(search): enforce recency across backends * docs(search): record recency provider contract	2026-08-30 21:25:05 +08:00
Yuzhong Zhang	8eda71fd97	fix(agents): normalize Command-wrapped tool results (#4977) ... * fix(agents): normalize Command-wrapped tool results Command-wrapped ToolMessages skipped result metadata and progress tracking, so error receipts could be recorded as success. * fix(agents): stamp error meta from subagent_status failures Delegated task Commands leave ToolMessage.status at success and do not use an Error: content prefix, so normalize_tool_message was labeling failed/cancelled/timed_out results as success. Honor structured subagent_status before content heuristics and cover the four statuses. * style: ruff-format tool_result_meta tests --------- Co-authored-by: Yuzhong Zhang <BetterAndBetterII@users.noreply.github.com>	2026-08-30 15:37:38 +08:00
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>	2026-08-30 15:29:51 +08:00
Wu Shuwen	a4e2a2b934	fix(sandbox): bound Windows command execution (#4946) ... * fix(sandbox): bound Windows command execution * fix(sandbox): preserve Windows output encoding * fix(sandbox): honor Python UTF-8 mode * fix(sandbox): normalize captured command newlines * fix(sandbox): scope newline normalization to Windows	2026-08-30 15:06:55 +08:00
Zeren Wang	22b0456e45	feat(harness): subagent report contract and delegation acceptance criteria (#5090) ... * feat(harness): subagent report contract and delegation acceptance criteria (RFC #4651 PR3) Layer 1 receipt verification is inert unless subagents actually cite their execution record. This lands the prompt layer that closes the adoption gap: - New subagents/report_contract.py owns the model-facing contract text, derived from the single-owner citation format (format_citation / receipt_id) so prompts can never drift from the verifier. The executor injects <report_contract> into every subagent system prompt — built-in and custom alike — requiring [rN tool_name] citations for action claims, verifiable handles (absolute path, URL, ID, HTTP status) for deliverables, and explicit failure reporting; the citation clause follows verification.receipts_enabled. - The task tool gains an optional keyword-only acceptance_criteria parameter, handed to the SubagentExecutor constructor and rendered into the subagent's SystemMessage (stripped, capped 20 items x 500 chars) — deliberately never the task HumanMessage, which InputSanitizationMiddleware classes as genuine user input and would HTML-escape into untrusted-input framing. The docstring frames subagent results as self-reports, states the citation cross-check's evidence boundary (resolved = the call happened, not that the claim is correct), and documents when to attach criteria with the canonical leaf forms. Deterministic leaf checking remains a separate layer. - The lead delegation workflow now instructs reading the ledger citation line as execution evidence only and spot-checking verifiable handles before synthesizing. - report_contract / acceptance_criteria are registered as blocked framework-authority tags in input sanitization so untrusted input cannot forge the verification contract. * fix(harness): neutralize acceptance criteria before system-channel injection render_acceptance_criteria_section interpolated lead-model-supplied acceptance_criteria verbatim into the subagent SystemMessage after only stripping/capping. A criterion such as '</acceptance_criteria><system>...</system>' could close the wrapper and open a framework authority tag, bypassing InputSanitizationMiddleware. Route each criterion through neutralize_untrusted_tags (the shared prompt-injection primitive) so blocked authority tags are HTML-escaped before interpolation. Add regression tests at the renderer and the executor _build_initial_state path. * fix(harness): keep model-supplied criteria off the system channel - Move acceptance_criteria values into the task HumanMessage — the untrusted channel InputSanitizationMiddleware escapes and boundary-frames. The subagent SystemMessage now carries only a framework-owned <acceptance_criteria> pointer note (no criterion text), so natural-language injection inside a criterion keeps task-data priority and cannot override framework instructions (PR #5090 review, willem-bd P1). - Condition the lead delegation workflow's citation verification guidance on verification.receipts_enabled and qualify the task tool's result-reading text with the enabled state, so a receipts-disabled configuration no longer tells the lead to require citation evidence that cannot exist (P2). * fix(harness): drop execution-record promise from report contract when receipts are disabled The <report_contract> opening was emitted unconditionally, so a verification.receipts_enabled=false subagent was told its report would be cross-checked against an execution record that cannot exist in that mode (terminal_receipts() returns None; no verdict, no ledger citation line). The opening now follows receipts_enabled: enabled keeps the cross-check language, disabled describes the handle-only review mode (PR #5090 review, willem-bd P2). * docs: record the prompt-layer trust-boundary self-check Generalizes the PR #5090 review outcome: before adding prompt text, ask of every data source in it what trust level it has and which channel it should ride — model/user-influenceable values ride the untrusted sanitized data channel, never framework-owned system text. Added to the PR template (Agents/LangGraph surface) and agents/AGENTS.md.	2026-08-30 11:39:25 +08:00
hataa	567a06783c	fix(mcp): tear down the in-flight owner when get_session is cancelled mid-eviction (#5008) ... * fix(mcp): keep session owner teardown safe across cancellation paths * fix(mcp): gate pooled-session publication on the commit inside the owner task The owner resolved `ready` as soon as initialize() finished, but the session only became pool property when the creator promoted it into _entries in Phase 4. A concurrent get_session() could join the in-flight creation and receive that session from Phase 2b while the creator was still parked in the Phase-2 eviction teardown; cancelling the creator then ran the Phase-2 unwind, which unconditionally shut the owner down — closing the session underneath the joiner (#5008 review). Move the commit into the owner task: initialize() success now pops the in-flight record, registers the session in _entries, and resolves ready with the session in one atomic critical section, so 'ready resolved with a result' is exactly 'session registered and pool-owned'. Joiners can therefore only ever receive a committed session, and both creator unwind paths (Phase 2 and Phase 3) skip teardown when the creation already committed, leaving the pooled session to LRU eviction / close_*. When the record was removed before the commit (close_* or creator unwind), the owner aborts and ready carries the same cancellation the old Phase-4 not-still-ours path raised, so joiners fail with the creation's outcome instead of hanging or holding an unmanaged session. test_cancelled_creator_does_not_close_session_held_by_joiner reproduces the review's scenario deterministically (MAX_SESSIONS=1, hung LRU victim, gated initialize, second caller receives the session, creator cancelled): red on the previous commit, green now. test_joiner_follows_creation_outcome_when_creator_is_cancelled pins the joiner outcome-gating semantics as a drift guard.	2026-08-30 11:36:26 +08:00
luo jiyin	0dd233afc4	feat(sandbox): make E2B mount upload deadline configurable (#4876) ... * feat(e2b-sandbox): make mount upload deadline configurable Replace the hardcoded 120-second mount upload deadline with a configurable `mount_upload_deadline_seconds` key read from SandboxConfig (extra=allow). The value is validated: zero and negative inputs are clamped to 1 second. Omitting the key preserves the existing 120-second default. This addresses the follow-up from PR #4842 review: operators with large mounts or slow networks can now size the deadline to their deployment without changing code. * fix(e2b-sandbox): address review feedback on configurable deadline - Remove import-time default capture from _mount_deadline_reason() and _MountUploadBudget.deadline_seconds to prevent silent drift. - Add warning log when mount_upload_deadline_seconds is clamped to 1 (was silent before). - Update AGENTS.md E2B Mount Uploads section: deadline is now configurable, not fixed 120. - Add mount_upload_deadline_seconds to YAML examples in provider docstring and __init__.py. - Add config-path test that exercises SandboxConfig -> _load_config -> _apply_mounts end-to-end. * fix(e2b-sandbox-provider): handle non-numeric mount_upload_deadline_seconds Guard _resolve_mount_upload_deadline against None, non-numeric strings, and other invalid values. None returns the default; non-numeric strings like '120s' or 'abc' log a warning and fall back to the 120-second default instead of crashing provider init with TypeError/ValueError. Extend the parametrized clamp test with None, suffix, and alpha cases, and add a warning assertion. Update CONFIGURATION.md with the new mount_upload_deadline_seconds key and its behavior. * fix(sandbox): handle infinite mount deadline	2026-08-30 11:31:53 +08:00
Wuong	e12925458a	feat(streaming): make heartbeat interval configurable (#5017) ... Co-authored-by: Wuong <26929475+Wuong@users.noreply.github.com>	2026-08-30 10:46:01 +08:00
hataa	2f8d1cfc21	fix(subagents): harden background-task registry and capacity snapshot edge cases (#5086) ... * fix(subagents): harden background-task registry and capacity snapshot edge cases - execute_async drops the just-registered background entry when submitting to the isolated loop fails. The caller sees the exception and never polls, and cleanup_background_task refuses non-terminal entries, so the entry would otherwise stay as a PENDING zombie forever. - SubagentExecutionCapacity.snapshot derives queued from len(_waiters) instead of iterating it. snapshot is read from non-loop threads (e.g. configure_subagent_execution_capacity) while the loop thread mutates the deque, so iteration can raise 'deque mutated during iteration'. The raw length may count a waiter that just timed out but has not removed itself yet, which only makes the busy-check more conservative. * fix(subagents): close failure-path gaps around background submit Address both review findings on the background submission lifecycle: - execute_async() copies the isolated-loop context before registering the _background_tasks entry, so a context-copy failure (callback- manager copy or loop-bound handler filtering) can no longer strand a permanent PENDING entry the caller will never poll. - _submit_to_isolated_loop_in_context() resolves the loop before calling the coroutine factory. As direct run_coroutine_threadsafe arguments the coroutine was created first, so a loop-startup failure stranded a never-awaited coroutine (RuntimeWarning + retained captures until collection). Both call sites share the fix. New tests verified red on the previous implementation, green after: - context-copy failure leaves no registry residue - the real submit helper (only the loop getter patched) never invokes the coroutine factory when loop startup fails * fix(subagents): close the coroutine when scheduling rejects it run_coroutine_threadsafe can itself raise once the coroutine exists (e.g. the loop closes between the lookup and the internal call_soon_threadsafe). Wrap the call, close the rejected coroutine, and re-raise; a focused test patches only run_coroutine_threadsafe and asserts the created coroutine reaches CORO_CLOSED.	2026-08-30 10:43:58 +08:00
Aniket Wagh	8a830f6354	fix(deps): depend on renamed tenki package instead of tenki-sandbox (#5087) ... * fix(deps): depend on renamed tenki package instead of tenki-sandbox tenki-sandbox has been removed from PyPI and republished as tenki. Its old wheel URL still resolves, so existing lockfiles keep installing and the breakage is invisible to anyone with a warm lock; any fresh resolution fails with 'tenki-sandbox was not found in the package registry'. tenki 1.0.2 still ships the tenki_sandbox module, so the imports in community/tenki/provider.py and sandbox.py are unchanged. Fixes #5081 * fix(tenki): point install guidance at the renamed distribution The rename to `tenki` left the user-facing remediation still naming the removed package. `_import_client` raised "pip install tenki-sandbox" on the missing-extra path — the exact instruction this change proves now 404s on PyPI, handed to the user at the exact moment they need it to work. Update that message and the remaining `tenki-sandbox` references in the provider, sandbox adapter, README, sandbox AGENTS.md and the test docstring. The imported module stays `tenki_sandbox`, so the distribution and module names now differ; each mention says so rather than just swapping the string. No behavior change beyond the error text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tenki): migrate the provider to the 1.x workspace-only API Renaming the dependency was not enough. tenki 1.0.2 keeps the tenki_sandbox module name but not its contract: Client.create dropped project_id and has no **kwargs to absorb it, and IdentityWorkspace no longer carries `projects` (the attribute is gone from the package entirely). Both configuration paths therefore failed before a sandbox could be created — explicit project scope raised TypeError, and automatic scope raised AttributeError walking workspace.projects. Scope is now the workspace alone. _resolve_scope returns a single workspace id, auto-selecting when the account has exactly one, and project_id is gone from create_kwargs and from the documented config surface. A stale project_id in config.yaml warns rather than fails. SandboxConfig is extra="allow", so simply not reading the key would leave it scoping nothing with no signal; it also used to short-circuit the identity lookup, so operators with more than one workspace need to know they must now set workspace_id. The suite passed against the broken provider because the fake client took **kwargs and swallowed the project_id the real SDK rejects. The double now mirrors 1.0.2 — keyword-only, no **kwargs — so an unexpected argument is a TypeError in tests exactly as it is against the SDK. Reintroducing the old create call fails 20 tests; before this change it failed none. Verified against the exact locked wheels: every other kwarg the provider passes (name, workspace_id, sticky, wait, max_duration, image, cpu_cores, memory_mb, env) and every SDK surface it touches (who_am_i, Identity.workspaces, wait_ready, exec, close, the fs API, the four terminal exception classes) is unchanged in 1.0.2. Reported by willem-bd in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(config): drop sandbox.project_id from the Tenki example The canonical example still documented project_id as a supported optional key after the provider stopped honouring it, so an operator following it could set the key, get no scope from it, and hit a workspace-resolution failure with nothing in the example to explain why. Replaced with a migration note rather than a silent deletion: someone upgrading already has the key in their config.yaml and needs to know it is inert now and that workspace_id is what scopes a sandbox on Tenki 1.x. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Aniket Wagh <aniketwaghh@users.noreply.github.com>	2026-08-30 10:37:58 +08:00
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.	2026-08-30 10:30:34 +08:00
Sunshine	bf740ffa90	feat(auth): add personal access tokens for programmatic API access (#5041) ... * feat(auth): add personal access tokens for programmatic API access (#4849) Backend-first implementation of the PAT contract from #4849: show-once dfp_ tokens bound to their owning user (AUTH_SOURCE_PAT, is_internal=false), digest-only storage (migration 0017), strict credential precedence (invalid Bearer is a 401, never cookie fallback), CSRF double-submit skipped only for Bearer requests while auth-endpoint origin checks still run, scopes intersecting the authz route permissions, session-auth-only PAT management and password changes, and throttled best-effort last_used_at stamps. * fix(auth): harden PAT scope boundary and schema parity from adversarial review Independent review of the initial draft found: (1) scopes only constrained the threads/runs permission axis while admin routes treated a PAT as its (possibly admin) owner — is_admin_user now rejects PAT callers outright since no scope grants admin capability; (2) the model declared a column UNIQUE constraint while migration 0017 created a named unique index, so downgrade failed on create_all-bootstrapped DBs — both now use the named unique index; (3) auth-disabled mode is an operator override and now stays ahead of the Bearer check so a stray Authorization header cannot 401 an E2E sandbox; plus wiring the previously-unused constants, bounding the last_used_at stamp cache, and four new tests (middleware-level expiry, expires_in_days, admin-capability rejection with session control, and the auth-disabled precedence). * docs(api): document personal access tokens for programmatic API access * fix(auth): close PAT security boundaries from review (default-deny routes, extension admin suppression) P1-1: scope intersection only constrains @require_permission routes, so undecorated mutation routes (DELETE /api/memory, POST /api/agents, Lark credential switching, channel config) accepted a PAT holding a single read scope. AuthMiddleware now enforces a default-deny route policy in auth/pat.py: PAT requests are admitted only to the thread/run lifecycle routes the v1 scopes govern; everything else answers 403 regardless of scopes. Session-cookie callers are unaffected. P1-2: the extension principal resolver projected is_admin/roles from the raw system_role, so an admin-owned PAT passed deerflow_extension_api.require_admin on contributed routes despite the documented no-admin guarantee. The projection is now PAT-aware and suppresses every admin signal for PAT callers, mirroring deps.is_admin_user. Both fixes carry regression tests (route outside policy 403 + session control; production resolver admin suppression), and API.md documents the default-deny boundary. * fix(auth): enforce PAT scopes on stateless run entry and harden decorator Follow-up hardening from an independent audit of the P1 fixes: - POST /api/runs/stream and /api/runs/wait were the only allowlisted run entrypoints without @require_permission, so a threads:read-only PAT could still start runs (same bug class as P1-1, now closed): both now carry @require_permission("runs", "create"). POST /api/threads and POST /api/threads/search gain threads:write / threads:read for the same reason. Authorization-disabled deployments see no change (the permission set resolves to all permissions). - require_permission now binds the wrapped signature to locate a positionally-passed request before injecting the test stub, fixing 'got multiple values for argument' on direct positional unit-test calls. - API.md: the intro PAT example used GET /api/models, which the new default-deny policy 403s — replaced with GET /api/threads; the default-deny route list now spells out method sets. Regression test: threads:read-only PAT is 403 on the decorated stateless entry while a runs:create PAT passes. * fix(auth): address review P2s (empty Authorization header, PAT name trimming, API example) - CSRFMiddleware treats an explicitly empty Authorization header as present (is None), so an invalid credential always reaches AuthMiddleware's uniform 401 instead of a CSRF 403 that varies by method/CSRF state. Regression: empty-header request dies at auth. - PATCreateRequest strips the name and rejects whitespace-only values before token generation; created names are stored trimmed. - API.md intro PAT example now uses the implemented POST /api/threads/search endpoint (GET /api/threads does not exist). - AGENTS.md trimmed back under the guidance soft budget after the upstream merge. * fix(auth): tighten PAT route policy to implemented methods only The allowlist admitted GET /api/threads, a method no router implements. Pre-authorizing a dead method weakens the default-deny boundary: a future GET collection route added without a permission decorator would become PAT-reachable without an explicit policy change. Restrict the rule to POST, fix the stale GET description in API.md's PAT constraints, and document the default-deny boundary accurately in the gateway AGENTS.md guidance (only the threads/runs allowlist is PAT-reachable; every other authenticated route 403s PAT callers). Audited every remaining rule against the mounted routers: all other method+path entries map to real routes. Regression: test_pat_policy_does_not_pre_authorize_unimplemented_methods. * test(auth): guarantee the negative digest test mutates the token token[:-1] + "X" is identical to the original whenever the generated token already ends in X (1/62), making the negative digest assertion fail intermittently. Choose the replacement character based on the existing tail so the mutated token always differs. * fix(auth): require runs:cancel for cancel-then-stream requests stream_existing_run is gated at runs:read so action-less stream joins work with read-only credentials, but its ?action=interrupt|rollback branch cancels the run — a separate permission. A runs:read-only PAT passed both the PAT route policy and the route decorator and could interrupt or roll back an active run, bypassing the runs:cancel scope. Decorators cannot express query-parameter-conditional permissions, so the check lives in require_cancel_permission_when_action(), applied at the top of the handler. Regression drives the real helper through the production middleware: runs:read-only PAT + action is 403, the same token joins action-less, runs:read+cancel passes, session control unaffected. * docs(changelog): add the PAT feature entry * docs(readme): add personal access tokens section Repo documentation-update policy requires user-facing features to update README.md in the same changeset; the PAT feature previously touched only backend/docs/API.md and the gateway AGENTS.md. * fix(auth): require runs:cancel for mutating multitask strategies All five run-creation entrypoints were gated only by runs:create, but RunCreateRequest.multitask_strategy accepts interrupt/rollback and start_run forwards it to create_or_reject, which terminates an already-active run. A runs:create-only PAT could therefore kill an existing run through a create request, bypassing runs:cancel. Decorators cannot express body-parameter-conditional permissions, and per-route checks leave the same hole for the next entrypoint, so the gate lives in start_run itself — the single choke point every run-creation path (HTTP routes and internal launchers) flows through. Regenerate launches pass multitask_strategy="reject" and are unaffected; requests without a stamped auth context (internal/test compositions) skip the gate. The check is the shared authz.require_cancel_permission_if primitive; require_cancel_permission_when_action now delegates to it, so every request dimension that carries cancel capability (query action, body strategy) flows through one gate. Regression drives the real middleware stack: runs:create-only PAT + interrupt/rollback is 403 with the exact detail, reject (explicit and default) stays available, runs:create+cancel passes, session control unaffected; a source anchor pins the gate inside start_run. * fix(runs): keep observer joins from applying creator cancel-on-disconnect sse_consumer's finally block applied the record's on_disconnect=cancel policy on ANY consumer's disconnect. The join surfaces (GET /join and the action-less GET/POST stream join) feed it the existing RunRecord, so anyone with thread read access — including a runs:read-only PAT — could cancel a locally-owned running run simply by closing the SSE connection, without runs:cancel. The policy expresses the creator's intent for their own connection; an observer's disconnect must never be read as that intent. sse_consumer gains apply_on_disconnect (default True). The two join surfaces pass False; the creating endpoints (thread-scoped and stateless create-and-stream) keep the creator semantics unchanged. wait_for_run_completion needs no change: its callers are creator-side or post-explicit-cancel paths only. Regression exercises a real generator close — the same machinery Starlette drives on client disconnect — against the production sse_consumer: creator stream disconnect cancels, observer join disconnect does not; a wiring anchor pins both join call sites and the creator defaults. API.md documents the cancel-capability constraint (this fix plus the action/strategy gates) in PAT Constraints. * test(auth): pin the multitask gate behaviorally; state wait invariant Independent adversarial review of the round-5 fixes found the P1-a regression only mirror-pinned: the source anchor could be satisfied by a comment, and deleting the gate from start_run would not fail the suite. This drives the production start_run directly — a create-only auth context gets 403 with the exact detail for interrupt, and a reject request with no cancel permission at all proceeds past the gate (never a permission 403). Also documents wait_for_run_completion's creator-side invariant (every caller is the creating endpoint or post-explicit-cancel) so a future observer wiring thinks twice before reusing it — the one-caller- away variant of the observer-disconnect P1. * docs(changelog): correct the PAT entry's digest and route-policy description The entry said HMAC digests (the implementation stores SHA-256 digests, as documented in API.md and pinned by the repository tests) and claimed the route policy admits 'implemented stateless endpoints' (it admits the thread/run lifecycle routes, narrowing further by scopes). Also notes the cancel-capability gate now covering action and multitask strategies. * fix(auth): enumerate the PAT runs route policy per implemented subroute The runs subtree rule was a GET|POST /runs(/.*)? wildcard — it pre-authorized every current and future subroute under /runs, including methods the router never implemented (e.g. GET /runs/stream), which is the same latent default-deny weakening the threads collection rule was tightened for: a future route added under /runs would become PAT-reachable without an explicit policy change. The wildcard is replaced with six segment-precise rules covering exactly the 14 implemented method+path combinations; the {run_id} slot necessarily matches any single segment, so the POST-only collection names (stream, wait, regenerate, edit-regenerate) are excluded from the GET run-id rule via negative lookahead — no dead method stays pre-authorized. Behavior for implemented routes is unchanged. test_pat_runs_policy_admits_exactly_the_mounted_routes derives the expected set from the mounted thread_runs router instead of a hand-maintained list: every implemented GET/POST route under /runs must be admitted, routes in this router outside the subtree stay denied, and representative unimplemented neighbors are denied — so adding a route under /runs now fails CI until it is explicitly allowlisted, and a removed route leaves a dead rule visible. API.md's PAT constraints list the enumerated routes and drops a feedback mention that belonged to the stateless /api/runs axis. * docs(migration): add the 0017 renumbering coordination note to 0017 The PR's migration-coordination comment states each migration file carries the note; the file did not. Adds it: numbering was generated against main head 0016 alongside #5078 and #4843; whoever merges first keeps the slot, the others renumber on rebase (revision/down_revision plus the bootstrap head assertions). * fix(auth): pad base62 tokens to a fixed 43-char width int.from_bytes discards leading zero bytes, so the unpadded encoder returned a variable-length body — empty for all-zero input, and shorter than 40 characters for any draw below 62**39 (~1 in 14.5M), leaving test_generate_pat_token_format probabilistically flaky and the token body without stable width (review round 6, P3). _base62 now left-pads with "0" to _base62_width(len(data)) — the exact integer digit count (62^43 > 2^256 > 62^42, so 43 for 32 bytes). The format test asserts the exact fixed width instead of a probabilistic floor, and a new unit test pins the all-zero, leading-zero-byte, and max-value edges deterministically.	2026-08-29 23:50:45 +08:00
Nefelibata	c6f6a01f56	fix(runtime): prevent IndexError in MemoryStreamBridge._make_gap on empty events buffer (#5047) ... * fix(runtime): prevent IndexError in MemoryStreamBridge._make_gap on empty events buffer * fix(runtime): handle empty stream replay gap bounds across backend and frontend - Clamp MemoryStreamBridge queue_maxsize at 1 and validate StreamBridgeConfig.queue_maxsize >= 1 - Update StreamGap docstring to clarify None retained bounds - Allow StreamReplayGapData and parseStreamReplayGap in frontend to accept string | null bounds, safely resuming when bounds are null - Add backend and frontend regression unit tests for queue clamping and null bounds replay gap * docs(stream-bridge): bump config_version and document empty buffer replay gap behavior * docs: document nullable gap bounds and sync helm config_version to 37	2026-08-29 17:21:29 +08:00
Zeren Wang	3b592c2053	feat(harness): subagent receipt citation verification (#5076) ... * feat(harness): subagent receipt citation verification - add receipt citation verification core - harvest subagent tool receipts at terminal status - transport subagent receipts and citation verdict via status contract - verify subagent report citations at task write-back - render citation verdicts in the delegation ledger * fix(gateway): strip forged receipt verdicts from the delegations channel normalize_input() and the checkpoint-state mutation sanitizer only stripped server-owned metadata from message-shaped values, so an external caller could submit a delegation entry carrying a forged receipt_verdict that render_delegation_ledger would present as runtime-owned execution evidence. Strip receipt_verdict from caller-supplied delegation entries on both the run-creation and thread-state mutation paths, with regression coverage for each. * fix(harness): close silent-pass gaps in the zero-citation heuristic The action-claim detector missed the most common completion verbs (fixed/added/tested/changed/...) and had no CJK coverage at all, so reports like 'I fixed the bug and added tests.' or '我已经创建了文件并运行了测试。' were treated as claim-free: citation_resolved=True with no ledger warning. Broaden the verb lists and add a language-independent safety net: when the run harvested receipts and a nontrivial (>=240 char) report cites none of them, flag it UNVERIFIED. Short claim-free confirmations remain a vacuous pass.	2026-08-29 16:56:56 +08:00
ChaseMoon	9c1dd11160	fix(mcp): preserve pooled stdio sessions after task timeouts (#5027)	2026-08-28 10:37:01 +08:00
Serply	4dbfe37ff3	feat(community): add Serply web search tool (#5023) ... Add deerflow.community.serply.tools:web_search_tool, a Google SERP provider for the web_search slot that also covers Google News and Google Scholar through an optional `vertical` config option. Reads the key from api_key in config.yaml or SERPLY_API_KEY, clamps max_results to Serply's 1-100 range, and returns the same structured JSON errors as the Serper and Brave tools. Register the provider in config.example.yaml, scripts/doctor.py, scripts/wizard/providers.py, .env.example, backend/docs/CONFIGURATION.md, the en/zh tools.mdx provider tabs, and tools/AGENTS.md. Tests mock httpx.	2026-08-28 10:30:30 +08:00
Terminator666666	e09b2d48df	fix(uploads): keep deduplicated filenames within the 255-byte limit (#5059) ... normalize_filename accepts names up to 255 UTF-8 bytes, but claim_unique_filename appended _N to the stem without re-checking the budget. A duplicate at maximum length therefore produced a 257-byte name, and the write path (open_upload_file_no_symlink -> normalize_filename) rejected it with ValueError. In the Gateway upload route that error falls into the generic handler: the whole request fails with a 500 and files already written in the same batch are rolled back — including unrelated ones. The same helper backs the Feishu and DingTalk channel downloads and client-side attachment staging. Truncate the stem on a UTF-8 code-point boundary when appending the dedupe tag would exceed 255 bytes, so the result always round-trips through normalize_filename. Names short enough to fit keep the exact dedupe shape they had before. Tests: red on main, green here — - unit: max-length dedupe stays within the limit and round-trips; repeated collisions stay unique; multibyte stems truncate on a code-point boundary; short names keep the historical _N shape - router: a batch with a max-length duplicate now succeeds and keeps every file instead of failing with a 500 Co-authored-by: Terminator666666 <Terminator666666@users.noreply.github.com>	2026-08-28 09:16:51 +08:00
georgelichen	24001e80b7	fix(skills): safely tokenize portable allowed-tools patterns (#4984) ... * fix(skills): accept portable frontmatter forms * fix(skills): normalize portable tool names * Safely preserve parenthesized portable skill tool patterns Portable Agent Skills declarations such as Bash(tvly *) contain spaces inside a command pattern. Keep those patterns as single literal entries while preserving exact names from the existing YAML-list form, so skill loading no longer fragments valid metadata or rewrites mixed-case MCP tools. Constraint: DeerFlow's current skill policy matches exact tool names and does not inspect Bash arguments Constraint: Agent Skills scalar syntax uses whitespace-separated entries with parenthesized command patterns Rejected: raw.split() | fragments Bash(tvly *) into unrelated tool names Rejected: normalize YAML-list entries | breaks case-sensitive MCP/runtime tool names Rejected: map Bash(...) to bash | broadens command-scoped declarations into unrestricted shell access Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Bash(...) entries literal and inactive until DeerFlow has an explicit command-pattern authorization model Tested: 175 focused parser, validation, installer, review, loader, and tool-policy tests; Ruff check and format; compileall; git diff --check Not-tested: Full backend suite stopped at pre-existing Windows mode assertion test_runtime_config_store_file_is_owner_only Related: #4912 * Preserve exact custom tool names in portable skill parsing Portable scalar frontmatter needs alias normalization for known DeerFlow-compatible names, but generic case conversion corrupts MCP and custom tool identifiers. The tokenizer also treated quoted or escaped parentheses as structural delimiters, rejecting valid command patterns. Preserve unknown names and parse quoted or escaped patterns without broadening Bash(...) into bash. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Parenthesized patterns remain literal because argument-level authorization is not implemented Rejected: Generic CamelCase-to-snake_case for every scalar | rewrites custom/MCP names Rejected: Map Bash(...) to bash | broadens command-scoped declarations into unrestricted shell access Confidence: high Scope-risk: narrow Reversibility: clean Directive: Add an explicit alias before supporting another portable tool name; keep command-pattern authorization separate Tested: 225 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite remains affected by unrelated Windows permissions/path and missing Lark CLI tests Related: #4984; #4912 * Preserve case-sensitive exact tool authorities Case-folding a scalar declaration before alias lookup can turn literal write into write_file, substituting a different runtime authority. Keep exact portable spellings as aliases and preserve lowercase, custom, and MCP names; strengthen activation coverage for spaced Bash patterns and command fragments. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Bash(...) remains literal and inactive because command-pattern authorization is not implemented Rejected: Case-insensitive alias lookup | maps lowercase runtime tools onto built-in authorities Rejected: Broaden the parser into command-pattern authorization | outside this PR's scope Confidence: high Scope-risk: narrow Reversibility: clean Directive: Add aliases only for documented portable spellings; preserve all other scalar names verbatim Tested: 226 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite remains affected by unrelated Windows permissions/path and missing Lark CLI tests; GitNexus index refresh remains stale Related: #4984; #5016297602 * Support portable Glob and Grep skill aliases Portable Agent Skills commonly declare Glob and Grep, but DeerFlow exposes the runtime tools as glob and grep. Add explicit exact-spelling aliases and activation coverage so imported skills retain search-tool access without broad normalization. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Alias conversion is limited to documented portable spellings Rejected: Case-fold all scalar names | can substitute custom or MCP authorities Rejected: Map arbitrary names by convention | breaks exact runtime compatibility Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep the alias table explicit and preserve unknown scalar names verbatim Tested: 228 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite has unrelated environment failures on Windows; GitNexus index reports stale line mappings Related: #4984; #5026257899 --------- Co-authored-by: kriptoburak <kriptoburak@users.noreply.github.com>	2026-08-28 08:58:59 +08:00
陈志谦	9e2c1be697	fix(sandbox): harden local Docker sandbox containers and port binding (#4986) ... * fix(sandbox): harden local Docker sandbox containers and port binding Root causes (security audit SBX-1/SBX-2) in the local container backend: - _resolve_docker_bind_host published sandbox ports on 0.0.0.0 whenever DEER_FLOW_SANDBOX_HOST was non-loopback (docker-compose defaults to host.docker.internal), exposing the unauthenticated /v1/shell/* exec API on every host interface. - _start_container ran every sandbox with seccomp=unconfined and no capability, privilege-escalation, or resource limits, so untrusted model-authored code could exhaust the host, escalate privileges, and reach internal networks / cloud metadata endpoints directly. Hardening changes and defaults: - Port binding: non-loopback sandbox hosts now bind the Docker default bridge gateway instead of 0.0.0.0, discovered dynamically via `docker network inspect bridge` with a static 172.17.0.1 fallback. host.docker.internal resolves to that gateway through host-gateway, so DooD gateways and the Docker host still reach the sandbox while external interfaces no longer see the port. DEER_FLOW_SANDBOX_BIND_HOST=0.0.0.0 restores the legacy broad bind. - seccomp=unconfined is no longer unconditional: sandboxes run with Docker's default seccomp profile; opt back in with DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED=1, only when the sandbox image is verified to require syscalls the default profile blocks. - Add --cap-drop=ALL and --security-opt no-new-privileges (Docker only; the Apple Container CLI does not support these flags). - Bounded resources with env overrides: --memory 2g (DEER_FLOW_SANDBOX_MEMORY), --cpus 2 (DEER_FLOW_SANDBOX_CPUS), --pids-limit 512 (DEER_FLOW_SANDBOX_PIDS_LIMIT); each also accepts "0"/"none" to disable the limit. - No --user is forced by default (the default AIO sandbox image's user is upstream-controlled and unverified), but DEER_FLOW_SANDBOX_CONTAINER_USER passes one through for deployments that know their image. - DEER_FLOW_SANDBOX_NETWORK passes --network so sandboxes can be attached to a dedicated egress-controlled network; default networking is unchanged. backend/docs/CONFIGURATION.md documents the new bind behavior and every override; tests cover each default and escape hatch. * fix(sandbox): follow host-gateway mapping for binds; keep image-required seccomp default Review follow-ups on the hardening change: - Bind: resolve the sandbox host itself and bind that address, instead of assuming the default bridge IPv4. host.docker.internal follows the daemon host-gateway-ip mapping (customizable, possibly IPv6), so the resolved address is exactly where the gateway connects — the published port and advertised URL always match. IPv6 is bracketed for docker -p, zone ids stripped, wildcard resolutions ignored; unresolved hosts fall back to the bridge gateway with a warning pointing at DEER_FLOW_SANDBOX_BIND_HOST. - seccomp: the shipped AIO image needs seccomp=unconfined for its Chromium browser (upstream quick-start always passes it; the upstream FAQ documents the browser failing under Docker default profile), so that option returns as the default. Tightening stays possible via DEER_FLOW_SANDBOX_SECCOMP_PROFILE=<path to a restricted, Chromium-compatible profile> or DEER_FLOW_SANDBOX_SECCOMP_UNCONFINED=0 for images verified to work with Docker's default profile. - cap-drop/no-new-privileges and the resource limits are unchanged. - Tests updated for both behaviors; 37 pass. * fix(sandbox): bracket bare IPv6 bind overrides; state seccomp default accurately DEER_FLOW_SANDBOX_BIND_HOST was returned verbatim, so a bare IPv6 literal like fd00::1 produced an invalid publish spec (fd00::1:port:8080); Docker requires the bracketed form. Normalize raw and already-bracketed IPv6 literals (IPv4/hostnames untouched), with resolver-level and argv-level tests covering the explicit IPv6 override. The CONFIGURATION.md overview claimed Docker's default seccomp profile stays active, contradicting the seccomp=unconfined default the table (and the code) actually ship for the Chromium-based image; spell out the relaxed default and where to change it. * style(sandbox): apply ruff format to local_backend * fix(sandbox): reject host networking, force builtin seccomp opt-out, resolve hostname binds Review follow-up on #4986 (willem-bd): - P1: DEER_FLOW_SANDBOX_NETWORK=host (and container:<name>) now raise a RuntimeError at start instead of silently voiding the hardened port bind — Docker discards -p/--publish in host mode and shares the network namespace for container:<name>, which would re-expose the unauthenticated exec API on the host's interfaces. Two regression tests cover both rejections. - P2: the seccomp opt-out now passes seccomp=builtin explicitly instead of omitting the option, so a daemon configured with an unconfined or custom default cannot weaken the documented opt-out; the test asserts the flag. - P2: hostname values in DEER_FLOW_SANDBOX_BIND_HOST resolve to an address before use (Docker publish specs require an IP literal as the host part, so host.docker.internal previously produced an invalid spec that prevented every sandbox from starting); unresolvable names raise a clear configuration error. Tests cover resolution and rejection; CONFIGURATION.md updated for all three behaviors. 43/43 pass in tests/test_aio_sandbox_local_backend.py; ruff check + format clean. * fix(sandbox): reject DEER_FLOW_SANDBOX_NETWORK=none (loopback-only, breaks published API port) * fix(sandbox): validate the effective Docker network target; normalize IPv6 sandbox hosts once name=host / name=none dodge raw-string checks but attach like the bare words; strip name= prefixes and validate the effective target (network IDs keep passing). Bracketed IPv6 sandbox hosts now resolve for the bind and bare IPv6 hosts produce bracketed URL authorities — both input forms give identical bind and URL addresses. * fix(sandbox): parse the full Docker network long syntax before validating Docker accepts comma-separated key=value fields in any order (name=, gw-priority=, alias=, ...); a name=host field hides the host network behind surrounding fields. Parse the CSV and validate the parsed name= target (last occurrence wins, fields lowercased, mirroring opts/network.go); no-name values fall through like Docker's own rejection. * fix(sandbox): keep CHOWN/SETUID/SETGID through cap-drop=ALL for the default image The shipped image's entrypoint starts as root, creates the gem user, chowns /opt/jupyter and drops to that user via su; without those three capabilities the set -e script dies before the readiness endpoint exists. no-new-privileges stays (it blocks gaining privileges via exec, not using the added caps). Adds a docker-gated real-image startup smoke test. * fix(sandbox): let pre-initialized non-root images drop the startup capabilities The CHOWN/SETUID/SETGID re-add only exists for the shipped image's root entrypoint handoff. A custom image that never runs as root gets an explicit opt-out (DEER_FLOW_SANDBOX_IMAGE_STARTUP_CAPS=0) so those capabilities are not left available to sandboxed code (chown on bind mounts, UID/GID impersonation). * test(sandbox): gate the real-image smoke test behind the live marker The default offline suite (make test = -m 'not live') must not depend on a third-party registry: mark the smoke test live, probe the daemon inside the test body (never at collection time), and allow pinning the image reference via DEER_FLOW_SANDBOX_SMOKE_IMAGE for a dedicated integration job. * test/docs: isolate DEER_FLOW_SANDBOX_IMAGE_STARTUP_CAPS in tests; add table row; split custom-image guidance _clear_hardening_env now clears the new knob so a developer shell or .env preset cannot flip the default-path tests. CONFIGURATION.md gains the table row, and the custom-image guidance becomes its own paragraph with the no-new-privileges scope stated correctly (it does not mitigate the retained CAP_SETUID/SETGID risk). * test(sandbox): make the live smoke test diagnosable 300s readiness budget (cold pull + cold start must not be conflated with broken capabilities) and dump the container's last 40 log lines on failure so the next live run tells us whether the capability set is incomplete (chown/useradd/su errors) or the services are merely slow. * test(ci): align the smoke test with the 60s provider deadline; add a dedicated live smoke workflow Single-source the readiness deadline as SANDBOX_LOCAL_PROVIDER_READY_TIMEOUT (used by both provider paths and the smoke test) so the validation cannot drift from the production contract again. New sandbox-image-smoke.yml runs the live test on a dedicated job, with the image reference pinnable via the SANDBOX_SMOKE_IMAGE repository variable (digest resolved and recorded in the job summary when falling back to :latest). * test(sandbox): pull the failing program's own logs on smoke failure supervisord only surfaces exit codes in docker logs; nginx's stderr lands in files inside the container. Dump supervisor program logs, nginx -t, and the nginx error log on failure so the next run names the exact broken line. * ci(sandbox): export an immutable repo@digest reference for the smoke run docker pull once on the runner platform, resolve RepoDigests[0], and pass that immutable reference to the test via GITHUB_ENV — the recorded and executed images can no longer diverge when the tag moves, and platform selection is left to the daemon instead of jq over the manifest index. * fix(sandbox): add DAC_OVERRIDE — the root nginx master writes gem-owned logs The image's root nginx master opens /var/log/nginx/{access,error}.log, which belong to the gem user, for the container's lifetime; without CAP_DAC_OVERRIDE it dies with 'open() failed (13: Permission denied)' on every start (FATAL under supervisord) and readiness never arrives. Four capabilities now: CHOWN/SETUID/SETGID for the entrypoint handoff plus this runtime log-write need.	2026-08-27 22:53:47 +08:00
yong	846c716523	feat(search): add Tencent Cloud WSA provider (#5057) ... * feat(search): add Tencent Cloud WSA provider * docs: restore README to upstream * docs: remove README changes from WSA provider PR * fix(doctor): validate Tencent WSA API key	2026-08-27 18:12:05 +08:00
青榆牧	a94b2d8897	feat(mcp): map request-scoped secrets to MCP HTTP/SSE headers (#5010) ... * feat(mcp): map request-scoped secrets to HTTP/SSE headers `user_auth` binds a credential to a configured DeerFlow user, so a caller that picks the credential per request — a multi-tenant gateway, a per-run API key, one shared MCP server fronting several environments — had to register one MCP server entry per credential. Add a declarative `mcpServers.<server>.headers_from_context` block mapping HTTP header names to keys of the run request's `config.context.secrets` carrier. A new built-in interceptor resolves the mapping on every tool call and rewrites those headers, mirroring `user_scoped_auth`. The config file stores names only, never a credential, so the Gateway returns the block unmasked. Registered after OAuth and `user_auth` in the interceptor chain: the later interceptor runs closer to the transport, and the value chosen for this one request is the most specific, so it wins. Fail-closed by default — a mapped key missing from the request raises a `ToolException` naming only that key, because falling back to the server's discovery credential would send one tenant's call under another tenant's authority. `on_missing: "passthrough"` opts out. Durable background tasks are excluded: `McpTaskToolCaller` drives status and cancel polls after the Agent run ends, where no run context exists, so the fail-closed interceptor would deny every poll. Those calls keep using server-level credentials, and a server declaring both `headers_from_context` and `task_toolsets` now logs a warning. Also corrects the custom-interceptor example in docs/MCP_SERVER.md (and the matching claim in skills/AGENTS.md), which read request secrets from `langgraph.config.get_config()["context"]`. That key is `None` inside a tool call — the run context rides the LangGraph runtime, not the RunnableConfig propagated to child runnables — so interceptors written from that example never saw a value. The example now reads `request.runtime`, and tests/test_mcp_context_headers.py pins LangGraph's runtime-injection rule by driving a real langchain-mcp-adapters tool through a real graph with the ambient-runtime fallback disabled. Closes #5005 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(mcp): resolve credential headers case-insensitively, carry them on durable submit Review follow-ups on `headers_from_context`. HTTP field names are case-insensitive, but every dict on the path to the wire is not: `build_server_params` copies the operator's static `headers` spelling verbatim, and langchain-mcp-adapters merges interceptor overrides into the connection with a plain `{**connection_headers, **override_headers}` splat. A static `authorization` and an injected `Authorization` therefore both reached httpx as separate field lines, and a server reading the field with a single-value accessor got the static discovery credential — inverting the documented `headers` < `oauth` < `user_auth` < `headers_from_context` precedence and running a per-request call under the shared credential. Normalizing inside the interceptor cannot fix that on its own: the adapter builds the request with `headers=None`, so an interceptor never sees the connection's static headers and cannot displace them however it spells its own key. A new `mcp/headers.py::apply_header_overrides` therefore drops any key differing only in case and emits the spelling the connection already uses. Applied to `headers_from_context`, `user_auth`, the OAuth interceptor, the OAuth discovery-header write, and the durable-task connection merge, which all carried the same collision. `headers_from_context.headers` now also rejects one header mapped under two spellings at config load, in both the harness model and the Gateway mirror. Durable submit now carries the mapped headers, as docs/MCP_SERVER.md already promised. `McpTaskToolCaller` disabled the interceptor for the whole caller, but that caller serves submit as well as the polls, and submit is awaited inline inside the Agent's tool call — where the run's LangGraph runtime is still the ambient contextvar, so no secret has to be threaded through `TaskSubmitRequest` or reach durable storage. The caller builds one chain and keeps a second view of it without the context-headers interceptor; `call_tool` takes `request_scoped_headers`, set only by `OrdinaryMcpTaskDriver.submit`. Status and cancel keep server-level credentials, so background polls still cannot fail closed, and the startup warning now describes the half it actually covers. `_merge_preserving_secrets` restores masked extras inside `headers_from_context` instead of writing the `***` sentinel back over the stored value, matching the treatment `user_auth` extras and server-level extras already get; extras a PUT omits carry over as well, while the declared mapping still replaces verbatim so a round trip can remove an entry. `extra="allow"` plus name-based sensitivity detection means the usual casualty is a name-valued key such as `tokenHeader`, not only a credential. The existing override test seeded the static header onto `request.headers`, which production never does, so it modelled a merge that really happens one layer down; the new tests drive a real adapter tool through a real connection and assert on the headers the session is opened with, and the durable-submit test runs through a real tool node with no runtime patching. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(mcp): reject case-insensitive duplicate static header names * fix(mcp): preserve omitted headers_from_context fields on partial updates --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 10:42:42 +08:00
Hyeonsang Cho	cb24bc2699	perf(middleware): stop checkpointing view_image base64 payloads (#5014) ... ViewImageMiddleware injected the viewed-image message from before_model and removed it again from after_model. before_model, model, and after_model are separate graph nodes, so every view_image turn cost two extra nodes and two state writes, and up to 20MB of base64 sat in two checkpoints for the duration of the model call. A run interrupted in that window (user cancel, restart) stranded the payload in history for good. Inject from wrap_model_call instead, so the message lives only in ModelRequest.messages and is never returned as a state update: - before_model/after_model (and the async pair) are replaced by wrap_model_call/awrap_model_call; _remove_image_context_messages and its RemoveMessage bookkeeping go with them. The async hook keeps the existing asyncio.to_thread offload for the file read and base64 encode. - _should_inject_image_message gates on request.messages rather than state, so the decision is made against what the model will actually see. - _inject sweeps this middleware's own message out of the request before rebuilding it. Dropping after_model also drops the cleanup it did on every call, so without the sweep a payload stranded by an older interrupted run would ride along in every later request for the life of the thread. Matching requires both the reserved id prefix and the server-owned marker, and Gateway strips that marker from client input, so a user message is never dropped. Chain position is unchanged, and wrap_model_call nests first-registered outermost, so TokenBudgetMiddleware still sees the image message and enforces the input budget against it. Checkpoint rows that already hold a stranded payload keep it on disk. It is inert -- never sent to a provider, and strip_data_url_image_blocks keeps it off the wire -- and reclaiming it would mean keeping the node this change removes. tests/test_view_image_middleware.py is rewritten around the new hook (43 tests): sync/async at unit and graph level, the stranded sweep, and the client-message protection. Docs: middleware chain entry 23, Vision Support, the middleware-execution-flow hook matrix and diagrams, and the strip_data_url_image_blocks docstring.	2026-08-27 09:12:37 +08:00