mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-26 14:36:19 +00:00
414 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
FanouZeng-TT
|
22ae3d0e95
|
fix(backend): validate assistant search pagination (#5506)
Co-authored-by: Claude Code <noreply@anthropic.com> |
||
|
NanPan
|
6a94bef908
|
fix(gateway): keep run drain alive across repeated cancellation (#5487)
* test(gateway): cover repeated cancellation during run drain * fix(gateway): keep run drain alive across repeated cancellation * test(gateway): harden run-drain cancellation coverage |
||
|
xiaodu55
|
e493390aea
|
fix(gateway): scope edit/regenerate helper fallbacks by data identity (#5483)
The message edit/regenerate prepare chain resolves source runs through _resolve_run_id_for_message, _require_successful_source_run and _find_interrupted_target_run_id, which filtered by the authorization identity (get_current_user) — the same conflation fixed for the runs and messages read endpoints in #5448, left out of scope there. For trusted internal callers the authorization identity never matches the raw owner-stamped run rows, so regenerate/edit-regenerate prepare fails with 409 on threads the caller is authorized on (#5482). The three helpers now resolve their filter id through _run_scope_user_id as well; both prepare endpoints keep their owner_check=True authorization and browser/API sessions keep the per-user filter. Regression tests extend test_thread_runs_internal_scope.py to the helper fallback paths: internal callers resolve raw-owner-stamped runs (including the interrupted-run and status-409 paths), browser sessions keep the per-user filter and 409 on cross-user runs. |
||
|
Zheng Haoran
|
4387dce7be
|
fix(channels): preserve thread-create lock generation (#5480) | ||
|
alanhuangyoo
|
0f2195e994
|
fix(goal): wait for the user when a turn ends on an unanswered question (#5467)
* fix(goal): wait for the user when a turn ends on an unanswered question ask_clarification and the sandbox network prompt put their question in a ToolMessage and end the graph. The goal evaluator only reads human and AI text, so it never saw the question, judged the goal not met, and the worker queued a hidden continuation telling the agent to keep going while the card was still open. The agent could then act on a guess before the user answered. Stand the goal down with blocker needs_user_input, without calling the evaluator, when the trailing tool results include a human input request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(goal): cover resuming after answered clarification --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aleksandr Sapronov
|
a922efe144
|
fix(channels): send Telegram messages as rich only when content has rich constructs (#5470) | ||
|
Xuehao Xu
|
f0cb67b223
|
feat(extensions): expose incremental run evidence reader (#5405)
* feat(extensions): expose incremental run evidence reader * fix(extensions): address run evidence review feedback * docs(extensions): clarify run deletion reconciliation * docs(migrations): align current head documentation * fix(extensions): isolate run evidence event reads * test: avoid pinning run change migration to latest head --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Zeren Wang
|
a58ab484a6
|
feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash (#5443)
* feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash Implements docs/superpowers/specs/2026-09-12-projects-mvp-phase2-design.md (issue #5160, tracker #5129) in the slice order of the spec's §16. Slices: - A: ProjectsConfig + write-time 422 UTF-8 byte cap; PROJECT_CONTEXT_KEY admission pinning (both server-owned sets + worker hoist); latest-only request-scoped <project> block via DynamicContextMiddleware wrap_model_call/awrap_model_call (idempotent reassembly, reserved ID prefix + marker + provenance, never persisted); journal audit fingerprints; Instructions tab. - B: ProjectDocumentRow + migration 0023; ProjectDocumentRepository with locked check-and-set; hash-qualified immutable shelf storage with Paths helpers; upload/list/content/delete-to-trash routes; project delete trashes the shelf in-transaction; request-scoped bounded <documents> index with honest count/shown + actionable overflow note; list_project_documents/read_project_document tools registered only on pinned runs; PAT allowlist + drift guards; blocking-IO anchors. - C: shared thread-upload ingestion service (uploads router refactored to parity); POST from-thread with provenance; attach-to-thread with lock-staged copy (archived source allowed); read-only thread-files view with per-group truncation reporting. - D: restore (restored/merged/not_found/no_target/content_missing; no file moves), purge (continuous row lock across unlink/delete/commit, retryable on FS errors), retention sweep (lazy + startup, 24h orphan guard, row-side reconciliation never deletes). - E: Documents tab (shelf + conversation-files browser, provenance, archived banner, content-missing rows), /workspace/trash route, sidebar entry, composer attach handoff, i18n (en-US/zh-CN), e2e mocks + specs. Review hardening folded in (10 rounds, all with tests): - force active shelf content (HTML/XML family) to download; nosniff on artifact + content responses; unified unsandboxed-iframe PDF preview (fixes the pre-existing Chromium sandbox blank in the artifact viewer) - scope document trash to the URL project under the document lock - atomic no-overwrite filename reservation for ALL ingestion (seeded claims + os.link commit with suffix retry; same-name re-upload now unique-names instead of replacing); hidden staging only, no visible placeholders; lease cleanup on setup failure - serialize conversion under the document lock with post-lock active revalidation; drain locked filesystem work on cancellation; preserve bytes when an insert's commit state is uncertain (including trashed rows) - original-integrity checks before serving text or cached conversions; content_missing surfaced in list responses (UI reads the flag, no 409-probe); downloads always serve original bytes - bounded streaming document reads with cached char counts; shelf limits declared in middleware release identity - thread-root confinement for from-thread sources; config fallback rejects fractional/infinite values; composer counts staged attachments; pending attachments persist until submission or removal; in-flight instruction/rename edits survive save refetches; shelf and trash pagination; conversation-file and thread-files pages stay subscribed to refetches Docs: README/README_zh, backend API.md/ARCHITECTURE.md, AGENTS.md contracts, config.example.yaml projects block. Review follow-ups (head b4807477 → this revision): - The trash retention sweep is split so repeated lazy triggers stay bounded: the indexed expiry purge still runs on every trigger (GET /api/trash/documents, POST /api/trash/purge) while the O(all rows + all files) reconciliation is throttled to one run per user per 15 minutes (process-local, per-user window). The startup sweep now runs as a background task instead of blocking gateway readiness, and shutdown awaits it (bounded). - The export scrub (stripInternalMarkers) is fence- and indentation-aware like the render path, so a pasted, fenced <project>/<documents> snippet survives markdown export while real injected blocks (never fenced) are still removed. Fence regexes moved to a dependency-free leaf module to avoid the messages↔streamdown import cycle. - The artifact viewer's PDF iframe no longer carries an added title attribute (the upstream e2e contract locates it via :not([title])), and the upstream artifact-preview spec now pins the new contract: PDFs render unsandboxed, images keep sandbox="". * fix(projects): round-2 review — cancel an overrun trash sweep, restore the PDF frame title - Shutdown cancelled only the shield around the background startup sweep, so an all-users reconciliation that outlived the 5s budget kept walking rows and files while the document repo and DB engine were disposed underneath it. The wait now lives in `_shutdown_startup_trash_sweep`, which cancels the task and drains it before worker exit: the shield keeps the wait bounded, the cancel makes it final (CancelledError lands at the sweep's next await, and `_run_startup_trash_sweep` only catches `Exception`, so nothing swallows it). - The browser-preview iframe lost `title={getFileName(filepath)}` in the previous fix round, leaving the PDF frame without an accessible name while its siblings keep theirs. Restore it (WCAG frame titles), assert it in the DOM test, and anchor the e2e on `iframe[title="report.pdf"]` instead of `iframe:not([title])`. * fix(projects): round-3 review — report the sweep's late finish, not a phantom cancel `Task.cancel()` returns False when the sweep already finished inside the window between the deadline firing and the cancel, so the shutdown log claimed a cancellation that never happened. Branch on that outcome: the warning stays for a real cancel, a late finish is logged at info, and both paths still reap the task before worker exit. * fix(projects): round-4 review — make Empty trash delete what it confirms `POST /api/trash/purge` only ran the retention sweep, and the sweep's candidate selection is age-gated, so a freshly trashed document survived "Empty trash" even though the confirmation promises that every listed document is permanently deleted. With one trashed row the route answered `{"purged": 0}` and left it in place; `GET /api/trash/documents` sweeps expired rows before listing, so the visible rows were normally ineligible for the action by construction. Empty trash now drives `purge_all_trashed`: the caller's trashed rows (`list_all_trashed`, no age filter) each go through the same guarded, row-locked `purge` as the single-document delete — bytes first, then the row, in one transaction — so a row restored mid-flight is skipped instead of force-deleted, and an unlink failure rolls that row back and answers 500 with a retryable message. Retention expiry stays where it was: the sweep's `purge_candidates` is now the only age-gated selection, and the lazy retention sweep still runs on the listing and at startup. Tests: the router suite replaces the retention-gated expectation with the reviewer's repro (fresh row purged, bytes unlinked, shelf and other users' trash untouched, a failing unlink stays retryable and 500); a blocking-I/O anchor drives the new entry point through the offload; the mocked e2e covers the action end to end; a new real-backend spec performs it against the real gateway and re-reads `GET /api/trash/documents`. README, API, ARCHITECTURE and the phase-2 design docs (en+zh) state the age-independent contract. |
||
|
hataa
|
26800d1245
|
fix(channels): stream-cap and validate WeChat/WeCom inbound media downloads, fixes #5223 (#5225)
* fix(channels): stream-cap and validate WeChat/WeCom inbound media downloads, fixes #5223 * fix(channels): address WeCom APPID, decompression, and log-sanitization review findings (#5223) Round-4 review follow-ups on the inbound-media download cap: - The COS bucket numeric suffix is the owner's Tencent Cloud APPID and bucket names are user-chosen, so any Tencent Cloud account could register a matching ww-aibot-img-* bucket and pass the shape gate. The built-in rule now admits only the APPID observed in Tencent's published aibot callback examples (1258476243), across regions; any other account (including a future WeCom rotation) goes through channels.wecom.allowed_media_hosts. - aiter_bytes() transparently decodes Content-Encoding, and the decoder allocates the full decompressed body before the byte cap sees a chunk (an ~8 KB gzip wire chunk decoding to 8 MiB reproduces it). Both URL readers now send Accept-Encoding: identity, refuse a response with a residual Content-Encoding before reading, and iterate aiter_raw(). - httpx.HTTPStatusError formats the signed URL (path + query credentials) into its message, so _ingest_inbound_files' reader-failure branch logs a sanitized summary (class + status) instead of logger.exception, and the WeChat extract paths catch httpx.HTTPError so the polling loop's per-message logger.exception can never render a media URL. Every change ships with a red/green regression: the reviewer's 403 mock-transport repro asserted against caplog.text (fully formatted logs), the reviewer's different-APPID bucket host, and gzip bombs driven through real httpx mock transports in both readers. Docs (channels AGENTS.md, README, config.example.yaml) updated for the APPID pinning and encoding gate. * docs(channels): document why the inbound-media cap is 50 MB, not WeCom's 100 MB ceiling * fix(logging): redact URLs in httpx request logs down to scheme + host, fixes #5223 httpx emits 'HTTP Request: GET <full URL>' at the Gateway's INFO level before any response handling runs, so even successful signed-media downloads leaked their credentials. HttpxUrlQueryRedactionFilter (installed by configure_logging) rewrites those records in place — path and query become /<redacted>, method/status/duration observability is preserved — which also keeps Telegram's token-bearing Bot API paths out of the logs. Reader-level regression tests run at production INFO level with a real MockTransport, success paths included. * fix(logging): blank userinfo credentials in httpx request-log redaction * fix(logging): redact authority-only URLs and cover urllib3 redirect logs Two follow-ups from the review plus one extrapolation of the same class: - rest is now optional in _URL_REDACT_RE, so an authority-only URL (scheme://user:pass@host, no path) is rewritten too — userinfo had nowhere else to hide and previously passed through verbatim. A bare credential-free origin still passes through unchanged. - Renamed to UrlRedactionFilter / install_url_log_redaction and attached to the urllib3 logger as well: urllib3 logs 'Redirecting <url> -> <url>' at INFO with full URLs on both sides, the same leak class on a different library logger. No gateway path today both uses requests and redirects a signed URL, but the class stays closed instead of dormant. - Unit tests now build records with the real httpx 0.28.1 format string ('HTTP Request: %s %s "%s %d %s"', 5 args) and httpx.URL args, per the nit, instead of a synthetic shape httpx never emits. * fix(logging): install URL redaction at handler level so propagated records are covered A logging.Filter on a logger only runs for records emitted through that exact logger — child loggers neither inherit it nor trigger it on propagation — so the previous attachment to the bare urllib3 logger was dead code: urllib3 emits Redirecting via urllib3.poolmanager at INFO and urllib3.connectionpool at DEBUG. The filter is now attached to every root handler (mirroring _install_trace_filter, which already iterates root handlers; handler-level filters see propagated records) in addition to the httpx logger (httpx emits via the bare name, and emission-point coverage survives handlers added later). The wiring is pinned by tests that emit through the real urllib3 child loggers — a mutation removing the handler-level install turns them red. Comments, docstrings, and AGENTS.md now state the actual emitter names and levels. * fix(logging): redact urllib3 DEBUG request lines, whose split shape evaded the URL regex urllib3's per-request line (connectionpool.py:545 on 2.7.0) renders as `scheme://host:port "METHOD /path?query HTTP/x.x" status len` — the authority ends at a space so _URL_REDACT_RE's bare-origin early return applies, and the quoted origin-form target has no scheme, so neither half was rewritten. UrlRedactionFilter now runs a dedicated request-line shape first (collapsing the target to /<redacted>, keeping scheme+host+method+ version), then the absolute-URL pass. Regressions pin the exact format string both at unit level and through the real urllib3.connectionpool DEBUG emit path; AGENTS.md wording now names both covered DEBUG shapes. * fix(logging): redact urllib3 retry lines and linearize scheme scanning Closes the two open review threads on the inbound-media log hardening: Retry/redirect targets: urllib3 logs the request target with no scheme in five shapes the generic absolute-URL pass cannot see - `Retry: <target>` (connectionpool.py:954 DEBUG), `Incremented Retry for (url='<target>')` (util/retry.py:545 DEBUG, absolute on the redirect path), `Retrying (...) after connection broken by '<err>': <target>` (connectionpool.py:869 WARNING, above the INFO root), and origin-form halves of both Redirecting emitters (poolmanager.py:500 INFO / connectionpool.py:922 DEBUG). Each gets a rewrite anchored to the exact urllib3 format, collapsing the target to /<redacted>; the generic pass's rest now stops at quote characters so a quoted URL keeps its closing punctuation (previously the absolute-form increment line was mangled), and the request-line method class accepts any case. The emitter enumeration in channels AGENTS.md is closed against the installed urllib3 2.7.0 source. Quadratic scanning: both scheme-bearing patterns start with a character class, so re.sub retried every suffix of a long token - 64K paths cost ~1.8s and URL-free 64K error bodies ~3.1s per record, synchronously in every root handler. The two passes are now driven from literal "://" occurrences: _scheme_starts walks back over the scheme charset to each run's first letter and the pattern is attempted only there, reproducing re.sub's leftmost-non-overlapping result in linear time (256K path: 5.6ms; worst adversarial shapes <= 28ms). Long-input regressions pin the URL-bearing and URL-free cases with mutation-verified bounds, plus nested-scheme and digit-headed-run equivalence cases. Validation: tests/test_logging_config.py 12/12; scheme-pass equivalence against the old re.sub pipeline verified by two independent 30k+ case fuzz runs; full-suite A/B against HEAD shows zero tests that pass on HEAD and fail with this diff. * fix(logging): boundary-aware quote stops and whole-message Redirecting anchor Two follow-ups on the urllib3 redaction shapes: Embedded quotes: `rest` treated ANY quote as a closing mark, so a URL with an apostrophe in the path kept everything after it verbatim (`https://h/path'quoted'?token=Q` rendered the credential suffix in full) while the class docstring claimed path/query/fragment are replaced. A quote now closes `rest` only at a boundary - followed by whitespace, a closing parenthesis, or end of string - so urllib3's Incremented Retry (url='...') scaffolding keeps its ') closer while an embedded quote stays consumed. The increment line's url capture gets the same rule narrowed to its fixed ')' closer. Redirecting anchoring: the origin-half pass matched `(? <=-> )/path` as a substring, and an `-> /path` arrow is not urllib3-owned shape - the sandbox provider's actionable mount error (`sandbox.mounts entry <host> -> /mnt/knowledge ignored: ...`) had its container path rewritten to /<redacted>, failing test_setup_path_mappings_logs_actionable_error_for_missing_host_path on CI (backend-unit-tests shard 3). The pass is now anchored to the whole `Redirecting <t> -> <t>` message, which is exactly urllib3's record; origin slots collapse, absolute slots stay for the generic pass. Regression tests pin the embedded-quote shapes and the sandbox error's byte-for-byte passthrough; both mutations verified red. Validation: tests/test_logging_config.py 14/14; the CI-failing sandbox test green locally; every test file asserting redaction/arrow log content passes (attachments, support bundle, run metadata, skill secrets, ragflow, skillscan, sandbox provider); full offline backend suite 14084 passed / 164 failed with the failure set matching this machine's documented Windows-environment baseline (NTFS chmod/symlink, docker/lark/langfuse absences) - no failure involves redaction output. * fix(logging): redact redirects with spaced locations * fix(logging): grammar-complete Redirecting anchor; neutral WeChat guard labels Round-13 P3 (Redirecting anchor strictness): the whole-message anchor kept the ^Redirecting prefix (the urllib3-owned literal that stops the sandbox false positive) but required BOTH slots whitespace-free, so a Location header with an interior space voided the pass and leaked the origin-form request target in the first slot - redirect_location is the raw header string and interior spaces are legal field syntax. The tail is now loose (\S.*$) and the first slot gets the same grammar treatment (\S.*?): the recursive urlopen frame passes the previous raw Location as its url, so t1 can carry interior spaces too, lazy-split at the first arrow the way the line is constructed. A space-carrying slot collapses whole when it starts with /; the sandbox mount error keeps passing through untouched. Round-14 nit (None conflation): _download_cdn_bytes returns None for two reasons (in-flight cap abort, Content-Encoding refusal) but both image and file callers labeled it "exceeds size limit (N bytes)" - contradicting the accurate encoding line right above it, and reporting the plaintext limit for a ciphertext-cap decision. Callers now log a neutral "skipped by download guard" line (the manager reader callers' shape); the accurate reason stays inside the download function. The same sweep also logs _stage_downloaded_file's silent None (no state dir configured), which made an attachment vanish with no log line at all. Also anchors the emitter-enumeration closure to its urllib3 version: the closure reopens if an upgrade changes these format strings, so the comment now says so explicitly. Validation: logging 15/15 and attachments 60/62 (the two pre-existing Windows symlink-privilege failures documented in the PR body); three mutations verified red (old wording, strict t1, silent staging None); ruff clean. Full offline suite run before push (per round-11 lesson). --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
xiaodu55
|
0745fb268f
|
fix(gateway): scope runs read endpoints by data identity, not authorization identity (#5448)
* fix(gateway): scope runs read endpoints by data identity, not authorization identity Trusted internal callers are authorized as a synthetic internal user (id "default", or the make_safe_user_id-normalized owner with an owner header), while start_run stamps run rows with the raw trusted-owner value. list_runs / get_run / list_runs_page filtered by the authorization identity, so the store-side user filter never matched and internal callers always saw an empty runs list (or 404) for threads they are authorized to read. The three read endpoints now resolve their filter id through _run_scope_user_id: internal-role callers skip the per-user filter (thread visibility is already authorized by owner_check=True), and browser/API sessions keep the existing per-user filter unchanged. Regression tests cover both identities across the three endpoints (list, keyset page, single get) with a MemoryRunStore seeded with mixed-owner rows; without the fix the four internal-caller cases fail while the browser-session isolation case passes. * fix(gateway): route the message read endpoints through the same data-identity scoping Review follow-up on #5448: list_thread_messages and list_thread_messages_page resolved get_current_user and passed it as the data filter to the event-store scan, hidden-run lookups, turn-duration injection and the feedback queries — the same authorization-vs-data identity conflation fixed for the runs endpoints, leaving the #5437 empty-read symptom in place for lossy owner values. Both endpoints now resolve their filter id through _run_scope_user_id as well. Regression tests extend to the two message endpoints, asserting the resolved filter identity at the runs-store and feedback-repo boundaries (None for internal callers, the session user id for browser sessions). * fix(feedback): deterministic per-run collapse for unfiltered feedback reads Review follow-up on #5448: with _run_scope_user_id returning None for internal callers, the feedback lookups now receive an explicit-None user id, which skips the user_id WHERE in FeedbackRepository. On shared/NULL-owner threads several browser users can hold feedback on the same run, and list_by_thread_grouped / list_by_run_ids collapsed rows per run_id via a dict comprehension over unordered results — the feedback attached to the last AI message would be an arbitrary user's row. Both methods now order by created_at ASC with feedback_id as the tie-break, so the collapse deterministically keeps the most recently created feedback. _run_scope_user_id's docstring now documents that the resolved id also scopes feedback and event-store reads, not just run rows. Regression test seeds multi-user feedback on one run and asserts the collapse outcome is stable across repeated unfiltered reads. * docs(feedback): the collapse keeps the most recently written feedback created_at is refreshed on upsert, so the surviving row per run is the most recently written (created or updated), not the most recently created — align both docstrings with the ordering key's actual semantics. |
||
|
Totoro
|
5b591a9039
|
feat(gateway): accept conversation references in run context and report the capability (#5463)
* feat(gateway): accept conversation references in run context and report the capability LangGraph SDK clients build a fixed run body and drop unknown top-level fields, so they cannot send the conversation_references field from #5399. RunCreateRequest now lifts context.conversation_references into the top-level field before validation, so it keeps the same bounds and error locations, and drops it from context, so it never reaches the merged run context or the checkpointed configurable. Sending both is a 422. GET /api/features reports conversation_references {enabled, max_references} with the same "tool is configured" predicate as run admission, so a client can hide an entry point on deployments without the tool. Related to #5398. * fix(gateway): report the field type error for a malformed top-level reference list A malformed top-level conversation_references sent alongside a context list now fails with the field's own type error instead of the conflict message. The tool-configured predicate reads tool.use directly, and the features test doubles carry that attribute like every real ToolConfig. * fix(gateway): treat every list-like top-level reference value as a conflict Pydantic's lax mode coerces tuples, sets, frozensets and deques into the list[str] field, so a direct Python caller passing one of those together with context.conversation_references now reports the conflict instead of slipping both grants through. Unreachable over HTTP, where JSON has no such types. * fix(gateway): ask pydantic whether a top-level reference value is list-like Enumerating list-like types cannot track pydantic's lax acceptance set (generators, UserList, dict key views also coerce into list[str]). The conflict guard now validates the top-level value with a TypeAdapter for list[Any]: whatever pydantic would coerce reports the conflict when it is non-empty, and whatever it rejects still surfaces the field's own type error. Regression tests cover deque, UserList, dict keys and a generator, plus rejected scalars. * fix(gateway): probe top-level references with the field's own annotation The conflict guard now validates the top-level value with the exact item annotation the field uses, so its acceptance set is the field's rather than a superset: an item the field rejects (an empty string, a non-string, the ints of a range or dict view) surfaces the field's own item error instead of a conflict. The annotation is shared through one alias so the two cannot drift. * fix(gateway): materialise a one-shot iterator before probing top-level references The item-validating probe could consume a generator while collecting an item error, after which the field re-validated the exhausted iterator, coerced it to [] and let the request through with the key still in context. Iterators are now read once into a list that both the probe and the field validate, so a bad item is reported at its index and a valid generator is kept. * fix(gateway): materialise every once-walkable iterable before probing references Pydantic coerces any iterable into the list field, and an object whose __iter__ hands out a generator once is not an Iterator instance, so the previous gate let it reach the probe and be consumed. The lift now reads every iterable except lists, tuples and the shapes the field rejects as a whole (str, bytes, dict) into a list first, so the probe and the field always validate the same items. --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> |
||
|
Kris
|
1d8b6ead0b
|
chore(discord): best-effort flush thread mappings on stop() + restart regression tests (#5461)
* fix(discord): flush thread mappings on shutdown to survive restart (#2897) The Discord channel already persists channel->thread mappings, but a mapping created right before a hard shutdown (process killed between thread creation and its background persistence write) could still be lost. Flush in-memory mappings in stop() as a best-effort safety net. Add regression tests covering the persist/load round-trip across a simulated restart and the stop() flush path. Refs #2897 * fix(discord): gate stop() flush on _thread_store_loaded (#5461 review) Address review feedback: - Only flush thread mappings from stop() once _load_active_threads() has run, so a stop() taken before the load (start() bailed on a missing bot_token / discord import error) cannot overwrite the store with {}. - Fix isort order in the test module (ruff I001). - Make the restart test a plain sync test (it never awaits). - Add a regression test proving stop() does not clobber the store before load. * fix(discord): gate stop() flush on _thread_store_loaded (#5461 review) Address review feedback: - Only flush thread mappings from stop() once _load_active_threads() has run, so a stop() taken before the load (start() bailed on a missing bot_token / discord import error) cannot overwrite the store with {}. - Fix isort order in the test module (ruff I001). - Make the restart test a plain sync test (it never awaits). - Add a regression test proving stop() does not clobber the store before load. --------- Co-authored-by: wcy12378 <wcy12378@users.noreply.github.com> |
||
|
PeaceMaker-best
|
14c9d44440
|
feat(runtime): persist tool-progress phase transitions (#5214)
* feat(runtime): persist tool-progress phase transitions Record bounded warn, block, and recover decisions for lead and task subagent runs while preserving event-loop isolation, fail-open behavior, and concurrent transition order. * fix(runtime): trust server-owned tool progress attribution * fix(runtime): centralize trusted audit attribution * fix(runtime): preserve complete tool progress audit state * docs: trim tool progress guidance to pass size check * fix(runtime): fence subagent audit recorder loop * docs(readme): sync tool-progress event coverage Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> --------- Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: 嗜鵼 <hy2010hy2010@qq.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Totoro
|
4ad55f598f
|
feat(conversation): continue reading a cut message by offset (#5434)
* feat(conversation): continue reading a cut message by offset A referenced message longer than 4,000 characters was cut, and its suffix could not be read back. Cut messages now carry a continuation (message_seq, offset). read_conversation(thread_id, message_seq, offset) returns the next part of that one message, sized to the same tool-output budget as pages. The read scans only the requested row under the existing visibility rules and rechecks ownership. Offsets follow the source's current text; an offset past the end is rejected. Related to #5398. * docs(conversation): say continuations ignore limit A continuation always returns one part of one message, so limit does not apply there. The tool schema now says so instead of discarding it silently. Related to #5398. * fix(conversation): stop instead of looping when no text fits the budget With a read_conversation tool-output budget below the envelope size, the fitted text was empty and the continuation repeated the requested offset, so an agent would repeat the identical call forever. Page and continuation reads now return output_budget_too_small with no continuation. Related to #5398. --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> |
||
|
Beautyl0ve
|
80f13935c2
|
feat(agents): allow custom agents to disable memory (#5167)
* feat(agents): allow custom agents to disable memory Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> * fix(agents): honor memory opt-out during compaction Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> * fix(runtime): preserve agent binding across state rewrites * fix(client): apply named-agent memory policy * fix(agents): address memory policy review feedback Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> * fix(agents): address remaining memory opt-out reviews Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> --------- Signed-off-by: Beautyl0ve <74452755+Beautyl0ve@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
alanhuangyoo
|
0a0d768107
|
fix(goal): stand the goal down once the run has hit its token budget (#5424)
* fix(goal): stand the goal down once the run has hit its token budget Since #5410 goal continuations share the run's token budget, so a continuation queued after the budget's hard stop only spends one more model call before its tool calls are stripped. Pass the run's stop_reason into the goal loop and stand the goal down with "token_capped" in that case. The evaluator still runs first, so a goal the capped run satisfied is cleared as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(goal): list the token-budget stop among goal-loop preconditions Also pin that a satisfied goal is cleared, not stood down, when the run hit its token budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Totoro
|
6177b07c06
|
fix(conversation): clarify reference semantics and keep reader pages inline (#5421)
Document that read permission expiry and source deletion do not erase text already copied into the destination conversation, and that reads follow the source's current visible history. Truncated results now tell the agent to acknowledge the omission and ask for the missing material before claiming every requirement is covered. Pages were filled to 20,000 text characters by cutting the last message that did not fit, and that suffix could never be paged back. They could also exceed the default 12,000-character tool-output budget, which externalized the page to a file. Pages are now sized by their serialized length against the read_conversation tool-output budget; a message that does not fit starts the next page intact, so only a message over 4,000 characters (or one whose escaped JSON alone exceeds the budget) is cut. Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
tiammomo
|
5d855e9b92
|
feat(scheduled-tasks): filter run history by occurrence status (#5384)
* feat(scheduled-tasks): filter run history by occurrence status Signed-off-by: tiammomo <26957354+tiammomo@users.noreply.github.com> * fix(scheduled-tasks): share occurrence status contract --------- Signed-off-by: tiammomo <26957354+tiammomo@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Totoro
|
533e30e7f2
|
[feat] add opt-in conversation reads for Gateway runs (#5399)
* feat: add scoped conversation reads to gateway runs * refactor: share conversation tool path and reuse parsed text --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> |
||
|
Hyeonsang Cho
|
bbced51022
|
fix(gateway): close two input-sanitization bypasses (#5375)
* fix(gateway): reject forged framework-injection markers in run input
`is_genuine_user_message` treats `hide_from_ui` and a human `name="summary"`
as proof the framework authored a message, and `InputSanitizationMiddleware`
skips those — escaping a real reminder's blocks would corrupt trusted context.
Neither marker was server-owned, so an external caller could set either one and
place a raw `<system-reminder>` outside the user-input boundary markers, which
the lead-agent system prompt declares trusted internal framework data. The
`hide_from_ui` variant is also filtered out of the thread UI, so the forgery
was invisible where it landed.
Both markers are now stripped from untrusted input, on the run path and on the
thread-state mutation route that writes straight into a checkpoint. Framework
injection happens inside the graph and never crosses this boundary, so nothing
the framework does is affected, and `trusted_internal` callers (IM channels,
the MCP task-notification launcher) keep writing hidden messages.
HumanInputCard replies are the one legitimate external `hide_from_ui`: the
frontend sends it alongside a `human_input_response` payload, so a message
carrying a valid one keeps the marker. That buys no bypass — the predicate
already classifies those as genuine, so they stay sanitized.
The name check reuses the predicate's own `_SUMMARY_MESSAGE_NAME` rather than a
fourth copy of the literal, and matches by `isinstance` exactly as the predicate
does: `HumanMessageChunk` is a `HumanMessage` whose `type` is not `"human"`, so
a type-based check would leave that subclass's marker settable. `name` is only
reserved on human messages — on a ToolMessage it is the tool's own name.
Three tests in test_gateway_services.py and test_message_provenance.py asserted
that a caller-supplied `hide_from_ui` survives. That assumption was the bypass;
they now pin the opposite, with a genuinely caller-owned key kept alongside to
prove the stripper is surgical.
* fix(agents): sanitize every genuine user message, not only the newest
The input guardrail scanned backwards for the first genuine user message and
returned, so only the newest turn was ever sanitized. The transformation is
request-scoped (`wrap_model_call`, never written to state), so thread state
keeps the raw text: once a newer turn arrived, the previous turn's payload was
replayed to the model verbatim, outside the boundary markers the lead-agent
prompt declares trusted framework data. The guardrail therefore held for
exactly one model call.
Reaching it needed no forged metadata and no crafted request body — type the
payload in one turn, then send anything at all in the next. A single request
carrying two user messages did it in one shot, since every message but the last
was skipped.
`_process_request` now walks the whole list and `_sanitize_message` owns the
per-message work; every existing branch (the `original_user_content` split for
upload turns, the multimodal rfind fallback, the metadata repair) is unchanged.
Framework-injected messages stay excluded by `is_genuine_user_message`.
Unexpected errors now fail open per message rather than per request. Iterating
history widened the old blast radius: one unprocessable row would have dropped
sanitization for the whole request, handing an attacker the newest turn by
crafting an older one. `GraphBubbleUp` still propagates.
Side effect worth noting: each turn's rendering is now stable across model
calls. Previously a turn was wrapped on its own call and unwrapped on the next,
changing the prompt prefix behind the newest turn and defeating prompt caching.
test_only_processes_last_user_message pinned the old scope; it now pins that
every turn is processed, and keeps driving the `wrap_model_call` entry point.
* docs: record the message-metadata trust boundary and sanitization scope
`agents/middlewares/AGENTS.md` owns the depth for InputSanitizationMiddleware
and documented only the `original_user_content` half of its trust boundary. Left
alone it would teach an agent that `hide_from_ui` is caller-owned and that the
guardrail covers one turn — and the usual failure mode is an agent "restoring"
the behaviour it believes was lost. The entry now carries both markers, the
HumanInputCard exception, the whole-history scope, and the per-message fail-open
rule.
The note lives only there. The root and `backend/AGENTS.md` layers are
orientation that points at the module guides owning the depth, and
`backend/AGENTS.md` is inherited by every backend chain — prose added there
inflates more than twenty of them, and `scripts/check_agent_guidance.py` shows
the middlewares chain has about a kilobyte of room against its hard limit.
CHANGELOG.md and CHANGELOG_zh.md record it under Security, continuing the
existing prompt-injection lineage.
* fix(gateway): mark caller-hidden messages instead of stripping the marker
Review follow-up. Stripping a caller-supplied `hide_from_ui` closed the bypass
but broke three frontend senders that use the marker purely to keep a context
message out of the transcript: the quoted conversation context
(`buildHiddenConversationQuoteMessage`), the sidecar context prompt
(`buildHiddenSidecarContextMessage`), and the agent save command. None carries a
`human_input_response`, so the HumanInputCard carve-out did not cover them, and
nothing else hides them — `_is_branch_visible_message` and the frontend's
`isHiddenFromUIMessage` both key solely on `hide_from_ui`, and no backend reads
`conversation_quote_context` or `sidecar_context`. All three would have rendered
as user-visible chat bubbles.
The marker plays two roles and only one of them is a vulnerability. The security
requirement is that a caller-supplied marker cannot skip sanitization, not that
it cannot hide a message. So the roles are separated instead of the marker being
removed: the Gateway keeps it and stamps the server-owned `UNTRUSTED_INPUT_KEY`,
and the guardrail now asks `requires_input_sanitization` — the mark, else the
genuine-user test. Hidden stays hidden; untrusted content is sanitized either
way. The reserved `summary` name is handled the same way and no longer rewritten.
Marking rather than removing is also the safer shape in general: `hide_from_ui`
is read for presentation, journal persistence, memory filtering and IM outbound
as well, and this boundary should not silently change any of them.
`is_genuine_user_message` is deliberately left alone. `ToolReceiptMiddleware`
uses it for turn-boundary detection, where a caller's hidden context message must
keep counting as not user-authored; widening it there would move the ledger's
turn window. `requires_input_sanitization` sits beside it in `message_utils` so
the two questions can be compared.
The three tests that asserted a caller-supplied `hide_from_ui` is removed now
assert it survives and carries the mark — which restores the original intent of
the two provenance cases, whose comment already read "caller-owned keys must
survive".
* docs(gateway): rewrite normalize_input's docstring around the mark
Review follow-up. The paragraph still described the pre-4a3344f6 strip model and
contradicted both the implementation and the middlewares AGENTS.md paragraph
updated in that same commit: it called `hide_from_ui` server-owned, said
carrying it skips sanitization entirely, and repeated the premise this branch
disproved — that HumanInputCard replies are the only legitimate external use.
It now describes what the code does: the markers stay caller-owned and are
preserved because three frontend senders rely on `hide_from_ui` for hiding
alone, the message is stamped with `untrusted_input` instead, and
`requires_input_sanitization` sanitizes it anyway. `untrusted_input` joins the
server-owned inventory in the preceding paragraph, which is what makes the stamp
unforgeable and unclearable.
The three surrounding docstrings now also say that these functions mark as well
as strip; `_strip_external_message_metadata` had advertised only the removal,
leaving a reader no way to find the stamp from the name.
* fix(gateway): mark state writes whose message omits additional_kwargs
Review follow-up. The state-route half of the fix missed the most natural
request shape. `_strip_external_metadata_from_message_like` returned early when
`additional_kwargs` was absent or not a dict — there was nothing to strip — and
that early return also skipped the mark. A `POST /threads/{id}/state` body of
`{"values": {"messages": [{"role": "user", "name": "summary", "content":
"<system-reminder>…</system-reminder>"}]}}` therefore reached the checkpoint
unmarked. The messages reducer's `convert_to_messages` then supplies
`additional_kwargs={}`, so at model-call time `requires_input_sanitization` fell
back to `is_genuine_user_message`, which a `summary` name fails, and the forged
tag reached the model raw and outside the boundary markers.
A missing or non-dict `additional_kwargs` is now treated as empty for both the
strip and the mark. The identity return is kept for the case where nothing
changes, so an ordinary key-omitted message does not gain an empty dict just by
passing through. The run path was never affected: `normalize_input` coerces to
BaseMessage first, which always carries the dict.
Every existing state-write test supplied an `additional_kwargs` dict, which is
why this shape slipped through; the regression now covers it at the route and
end to end through the reducer into the guardrail.
While checking the neighbouring shapes, `_skips_input_guardrail` keyed off key
presence where `is_genuine_user_message` keys off truthiness, so
`hide_from_ui: False` — already covered without a mark — would have been
stamped. It now mirrors the predicate exactly, as its docstring claimed.
* docs(middlewares): compress the sanitization note to fit the guidance chain
main's growth left the middlewares AGENTS.md chain 84 bytes under its hard
limit, and the fuller wording did not fit. The load-bearing facts stay — the
markers are marked rather than stripped, and the scan covers every turn — since
those are the two an agent editing this middleware could otherwise get wrong.
The full model lives in the normalize_input, _mark_untrusted_framework_markers
and requires_input_sanitization docstrings.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
yang rui
|
7513f16e0e
|
feat(settings): persist account preferences across browsers (#5397)
* feat(settings): persist account preferences across browsers * docs(settings): scope preference guidance to user persistence * fix(settings): preserve SSR and fence custom-agent defaults * test: include user persistence in scoped guidance inventory * fix(settings): sync explicit edits and preserve local tab updates |
||
|
Xuehao Xu
|
56540fab01
|
fix(gateway): make recursion limit configurable (#5390)
* fix(gateway): make recursion limit configurable * docs: keep backend guidance within inherited size budget * fix(gateway): address recursion limit review feedback * docs(gateway): clarify recursion default scope --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Zhengcy05
|
81f2015fe6
|
fix(runtime): keep agent construction off event loop (#5217)
* fix(runtime): keep agent construction off event loop * fix: - offload checkpoint state accessor graph construction to a worker thread - update test * import AsyncKeyedLockTable * update Agents.md * fix: update test * fix: preserve single-flight builds after cancellation --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Wenchao An
|
1b76ab9060
|
feat: add opt-in task notes and compacted history recall (#5382)
* feat: add opt-in task notes and compacted history recall * fix: validate task continuity state and preserve user answers Honor explicit opt-out, preserve clarification replies and capture failure statuses, validate notebook writes, and clear branch archive references. Update the config version and audit optional LLM credentials, with regression and integration evidence. * fix: align Helm config version with task continuity schema * fix: preserve mixed task history and declare continuity policies * fix: recover malformed history and evict archives atomically |
||
|
tiammomo
|
444bfb72ce
|
feat(scheduled-tasks): preview upcoming cron occurrences (#5381)
Signed-off-by: tiammomo <26957354+tiammomo@users.noreply.github.com> |
||
|
Hyeonsang Cho
|
bc4a33aba7
|
fix(skills): stop persisting resolved secrets when toggling skills (#5357)
Toggling a skill wrote resolved secrets into extensions_config.json. The Gateway skill toggle and DeerFlowClient.update_skill loaded the file with ExtensionsConfig.from_file(), which replaces every "$VAR" string with the environment value (and an unset variable with ""), then serialized that model back through to_file_dict(). A "$GITHUB_TOKEN" reference was persisted as the plaintext token, and an unset reference was erased for good. DeerFlowClient.update_mcp_config had the same flaw for every key other than mcpServers. Every writer now does a raw read-modify-write, the way the MCP router already did: read_raw_extensions_config reads the on-disk JSON, set_raw_skill_enabled changes only the target entry, and validate_raw_extensions_config checks the candidate the way the runtime will load it before the atomic write. When the file does not exist yet, the Gateway seeds it with the cached skill states only, never the resolved cached model. The MCP router's raw loader and candidate validation delegate to the same helpers, so the rule lives in one place, and to_file_dict() is removed so the unsafe serialization has no entry point left. Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
yang rui
|
ac2b6415ea
|
feat(agents): support Unicode display names for custom agents (#5324)
* feat(agents): support Unicode display names for custom agents * fix(agents): preserve and validate Unicode display names * fix(agents): tolerate invalid stored labels and reject invisible names * fix(agents): identify agent in invalid display name warning * style(frontend): format agent display name fallback --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Yufeng He
|
3aa1ac477d
|
fix(channels): cap WeCom outbound content at the 20480-byte protocol limit (#5148)
* fix(channels): cap WeCom outbound content at the 20480-byte protocol limit Both _send_ws paths sent unbounded text. Stream replies now clip on a character boundary with a truncation marker (one stream carries the whole reply and cannot split mid-way), and proactive pushes split into sequential markdown messages at newline boundaries. Measured in UTF-8 bytes, matching the documented protocol cap. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * test(channels): pin emoji boundary behavior in the WeCom content limit Review on #5148 raised 4-byte emoji cut points. Probes show the split path already carries a byte-split character into the next chunk and terminates on all-emoji input; these tests pin that behavior so a later refactor cannot regress it. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(channels): preserve the delimiter when splitting WeCom pushes The boundary newline was stripped by lstrip, so the sequential markdown messages lost one delimiter per split and could not rebuild the original response. Keep it on the chunk's tail and assert the exact round trip in the tests, including leading blank lines. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(channels): serialize WeCom proactive chunk batches per chat Each chunk send awaits, so two manager workers pushing long texts to the same chat could interleave markdown messages (A1, B1, A2, B2) and break the sequential-message contract. Hold a per-chat lock across the whole split batch; different chats still send concurrently. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(channels): keep WeCom split advancing and cap the chunk batch Two edge cases in _split_for_byte_limit left after the delimiter fix: - A limit narrower than one whole character made the decode window empty, so the hard cut became 0 and the loop appended empty chunks forever. Take the character anyway when the window decodes empty, so the loop always advances. - A single oversized push produced one message per 20480 bytes with no ceiling, flooding the chat and holding the per-chat lock for the whole drain. Cap one push at 10 messages: keep the first nine verbatim and collapse the rest into one clipped tail carrying the truncation marker, with a warning log when the cap trips. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * fix(channels): reclaim completed WeCom send locks, cap the split before it does the work Two leftovers from the last review round: - _ws_send_locks kept one lock per chat forever. A guard-locked refcount now reclaims an entry only when no sender is queued on it, so a waiter can never land on a fresh lock mid-batch for the same chat. Pinned by three reclamation tests (single push, capped batch, 20 concurrent chats). - _split_for_byte_limit built every chunk and then joined the discarded tail to clip it — quadratic work on pathological pushes. The batch cap now applies inside the loop: once the kept chunks are full, the remainder is clipped whole. A spy test pins that the clipper receives the unsplit remainder. tests/test_wecom_content_limit.py 25 passed, plus tests/test_wecom_ws_text.py. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> * docs(channels): describe the WeCom push cap, pin the staggered-lock race The channels guide still described proactive pushes as an uncapped split. Also add the staggered-start regression the reviewer asked for: a waiter queuing while the holder's cleanup runs must share one lock, keep batches contiguous, and leave both lock registries empty. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> --------- Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com> |
||
|
Hyeonsang Cho
|
806a5bd427
|
fix(gateway): serve XML artifacts as attachments to block same-origin script (#5353)
* fix(gateway): serve XML artifacts as attachments to block same-origin script
GET /api/threads/{id}/artifacts/{path} forced only text/html,
application/xhtml+xml and image/svg+xml to download. Every other XML
document was served inline from the application origin: `.xml` guesses
to text/xml or application/xml depending on the host's mime.types, and
both fell through to the inline text branches. Browsers render any XML
MIME type as a document and run an XHTML-namespaced <script> inside it,
so a report.xml written by a prompt-injected agent and opened from a
chat link executed with the viewer's session: the HttpOnly access_token
rides same-origin fetches, and the double-submit csrf_token cookie is
JS-readable, so state-changing calls are reachable as well.
Treat HTML plus every WHATWG XML MIME type (text/xml, application/xml,
any +xml subtype) and text/xsl, which Blink also renders as XML, as
active content. A single helper owns the rule for both the regular-file
and the .skill-archive-member branches. The artifacts panel already
previews .xml as code through a ranged fetch, so preview and editing
keep working against the attachment response.
* docs(frontend): name XML among the artifacts the Gateway downloads
Review follow-up on #5353: resolveArtifactOpenURL's comment still named
only HTML/SVG as the active content the Gateway serves as a download.
XML documents now join that bucket, so the frontend note matches the
Gateway rule. Comment-only; no behavior change.
|
||
|
Shxiao
|
572744975d
|
fix(tools): run tool assembly off-loop at async entry points (#5224)
* fix(tools): run tool assembly off-loop at async entry points get_available_tools() may block on MCP cache initialization while it is called on async agent-assembly paths (task_tool, durable batch execution), stalling the calling event loop for the full discovery duration. Dispatch the (unchanged, synchronous) assembly call to a worker thread via asyncio.to_thread at the two async entry points so the loop keeps processing requests, SSE frames, cancellations, and timers. Fixes #5172 * fix(tools): offload lead-agent assembly off-loop and pin with blocking-io anchors Review follow-up for #5224: - run_agent now dispatches agent_factory(...) through asyncio.to_thread, so lead-agent assembly (including both get_available_tools call sites in _assemble_lead_agent) runs off the event loop — the Gateway headline scenario from issue #5172. - _ensure_sync_invocable_tool takes a double-checked threading.Lock, making the in-place tool.func wrap on the shared tool singletons explicitly single-shot now that assembly can run concurrently on worker threads. - Add backend/tests/blocking_io/test_tool_assembly_offloop.py: blocking-probe anchors for task_tool and SubagentBatchService._execute_item under the strict Blockbuster gate, plus a meta-check proving the gate trips on the exact syscall class (ExtensionsConfig.from_file on the loop). Verified the anchor goes red when the offload is flattened back to a plain call. * fix(gateway): build checkpoint state accessor off-loop; anchor run_agent offload Review follow-up for #5224: - Add abuild_checkpoint_state_accessor (asyncio.to_thread around the unchanged sync builder) and switch every async call site to it: the stateless_wait route, thread_runs, both threads call sites, and the build_thread_checkpoint_state_accessor boundary. The agent-factory assembly re-enters get_available_tools() and may block on MCP cache initialization; repeat calls hit _state_accessor_graph_cache and only pay the thread hop. - Add a third blocking-io anchor driving the real run_agent with minimal RunManager/bridge stubs; the factory performs a real production blocking read (ExtensionsConfig.from_file()) and the test asserts assembly never runs on the main thread. Verified the anchor goes red when the run_agent offload is flattened back to a plain call. - Adapt the test_threads_router checkpoint-builder patch sites to the new async name. * refactor(tools): carry assembly offloads on a dedicated bounded pool Review follow-up for #5224: - Add utils/assembly_io.py: a dedicated ThreadPoolExecutor (default 8 workers, DEER_FLOW_ASSEMBLY_WORKERS-overridable, mirroring utils/file_io.py and tools/sync.py) with run_assembly(), which copies contextvars explicitly. A hung stdio MCP server parks its worker for the full MCP timeout; carrying assembly hops on the loop's default executor would let a few parked assemblies queue every other to_thread/run_in_executor(None, ...) caller behind them. - Switch all four offloads (run_agent, task_tool, batch _execute_item, abuild_checkpoint_state_accessor) to run_assembly(). - State the cold-path behavior in the accessor docstring: the graph cache validates factory identity, so non-identity-stable factories may duplicate lead-agent assembly across concurrent readers (MCP discovery stays process-wide single-flight); the pool bounds the duplicates. - Add a fourth blocking-io anchor driving build_thread_checkpoint_state_ accessor with a per-resolution fresh factory (always a cache miss) and the real production blocking read; enumerate all four offloads in the gate's module docstring. Verified the anchor goes red when abuild_checkpoint_state_accessor is flattened back to a plain call. * fix(subagents): revalidate batch item before launch; make assembly pool observable Review follow-up for #5224: - _execute_item() revalidates the durable state right after assembly and before executor.execute_async(): renew_item_lease() returns valid=False when cancel_batch() terminalized the item or the lease was lost while assembly was parked, and the launch is skipped (the canceller already finalized the item). Previously the launch was unconditional and the poll loop's cancellation checks only started after execution began. - Regression test driving the real SQLite repository: a blocking assembly probe parks _execute_item, cancel_batch() lands, and the launch is skipped with the item staying cancelled. Verified the test goes red when the revalidation is removed. - run_assembly() tracks pending assemblies and logs a throttled WARNING once the pending count exceeds the worker count, so assembly starvation (workers parked on a hung MCP server) is distinguishable from idle. - The run_agent blocking-io anchor now binds a sentinel extension snapshot via ctx.extensions and asserts the factory observed it through get_agent_build_extensions(), pinning run_assembly()'s ContextVar propagation. Verified red when ctx.run is dropped. - Document the assembly pool in backend/AGENTS.md. * fix(utils): decrement the assembly pending count on the pool thread The pending-assembly counter behind the starvation warning decremented from the asyncio future's done callback, which never fires once the submitting loop is closed while its worker is still running: the count ratcheted up permanently and eventually fired the starvation warning with no starvation behind it (reproduced at 97dc9bec by review). Decrement instead from the dispatched work item: run_assembly() wraps func so a finally drops the count under the pending lock on the pool thread, and the done callback is gone. Pin the counter with tests/test_assembly_io.py: a healthy call returns the count to zero, and an abandoned loop (stopped while the worker is parked) does not wedge it — the abandoned case goes red against the old done-callback decrement. * docs(utils): fix the pending-counter comment after the decrement move The comment still described the removed done-callback decrement, contradicting _work()'s own comment; state the actual mechanism (increment on the loop before dispatch, decrement from the dispatched work item's finally on a pool thread). * test(gateway): retarget checkpoint-accessor stubs to the services seam thread_runs and runs now call abuild_checkpoint_state_accessor, so the upstream wait-reader, regenerate-prepare, and idempotency tests must stub the sync builder where abuild resolves it (app.gateway.services); stubbing the removed router re-exports fails with AttributeError at setup. The async seam semantics are unchanged: run_assembly invokes the stubbed sync builder off-loop and propagates its return values and exceptions. Move the agent/tool assembly off-load note from backend/AGENTS.md to deerflow/utils/AGENTS.md (next to assembly_io.py) so the effective instruction chain for agents/middlewares no longer grows past the AG002 hard limit. * fix(runtime): serialize same-key accessor assembly and release queued-cancel slots Address the three review follow-ups on the assembly off-load: - assembly_io: a job cancelled while still queued never runs its work item, so the dispatched finally never fired and _pending_assemblies stayed elevated until a false starvation warning. Exactly-once cleanup now rides the concurrent future's cancelled() state — cancel() only succeeds before the executor starts the item, so cancelled() is true precisely when the finally will never run — plus a submit-failure release; the one-worker queued-cancellation case is pinned red/green. - services: overlapping cold readers sharing one cache key could both run full agent assembly. _state_accessor_graph now serializes per key through a thread-side KeyedLockTable (pool threads, no running loop) and re-validates factory/app-config identity under the lock, so the factory runs exactly once while identity changes still rebuild. Cache dict access is lock-guarded now that construction runs off-loop. - guidance inventory: register deerflow/utils/AGENTS.md in EXPECTED_GUIDANCE_PATHS so test_repository_has_the_approved_scoped_ guidance_shape matches the relocated assembly note (CI shard 4). * test(keyed-lock): pin KeyedLockTable reclamation and waiter bypass directly Thread-side counterparts of the async table's own tests: overlapping hold() calls serialize (a late arrival joins the live entry instead of creating a second lock that bypasses a queued waiter), the last check-in pops the entry, and many unique keys leave the registry empty. Both regressions verified red — popping unconditionally trips the late-arrival test, never reclaiming trips the many-keys test. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Totoro
|
cd0e74edaf
|
fix(scheduler): reconcile stuck once tasks from committed run outcome (#5035)
* fix(scheduler): reconcile stuck once tasks from committed run outcome Restart recovery (cancel_stuck_once_tasks and the multi-instance reconcile_stuck_once_tasks) blindly flipped every stuck once-task to 'cancelled'. When handle_run_completion crashed between its two transactions, a once-task whose run had already committed 'success' was permanently reported as cancelled. Both reconciliation paths now read the latest scheduled_task_runs row without a status filter and finalize the parent to match: success -> completed (last_error cleared), failed -> failed with the run's error, interrupted -> cancelled with the run's error when present, skipped -> cancelled (no work performed). Active occurrences (queued/launching/running) are left untouched — a concurrent completion or a later recovery pass will finalize them once the run reaches a terminal state. Tasks without a terminal run row keep the previous generic cancellation. Review follow-ups (willem-bd / Huixin615): - Extract _finalise_once_task_from_run() so both recovery paths share one outcome mapping (no more drift between single- and multi-instance paths). Returns bool (True = finalised, False = active/no-op) for explicit counter management at call sites. - Fix a no-op (`run_row.error or None` -> `run_row.error`) in the skipped branch. - Drop the unused `status` parameter from the test task helpers. - Use TERMINAL_RUN_STATUSES / ACTIVE_RUN_STATUSES constants (local copies to avoid circular import; kept in sync with scheduled_task_runs.sql). - [P1] Read the latest run AFTER acquiring the parent task row lock, not from a pre-lock batch snapshot. The latest-run lookup now runs per task under the lock with populate_existing so a concurrently committed status is read back fresh. - [P2] Race tests now use monkeypatch to actually enter the race window: _intercepted_fetch commits success in a separate session at the moment the per-task fetch fires, so a reverted pre-lock batch implementation fails the test, while the current post-lock implementation passes. - [P1] Do not finalize parent for active occurrences. A non-terminal scheduled occurrence means the run is still in progress — the parent must be left untouched until the completion path or a later recovery pass establishes a terminal outcome. - [P2] Add cancel_stuck_once_tasks to the single-instance poll loop so stuck once-tasks are not left permanently "running" when the startup sweep fails (mirrors multi-instance _reconcile_active_state behavior). - Fix stale docstrings in cancel_stuck_once_tasks and _fetch_latest_run. Adds regression tests for multiple historical runs (older success + newer skipped/active) on both paths, monkeypatch-based race tests that prove a concurrent completion committing success is reflected as completed, and active-run tests that verify the parent is left unchanged. Documents the behavior in AGENTS.md. Fixes #5034 * fix(scheduler): address review comments on completion-consistency fix - _fetch_latest_run: drop arbitrary id DESC tie-break; order by scheduled_for DESC (deterministic recency on schedule position) - _finalise_once_task_from_run: annotate bool return type - Centralize TERMINAL/ACTIVE_RUN_STATUSES in scheduled_tasks/model.py; stop duplicating them in scheduled_tasks/sql.py and scheduled_task_runs/sql.py (removes stale circular-import workaround) - cancel_stuck_once_tasks: run unconditionally in single-instance poll loop (remove try/except swallow) - tests: pin created_at/scheduled_for in _create_run so recency ordering is actually exercised; correct docstrings that described the active-occurrence branch as 'generic cancel' instead of 'left unchanged' * fix(scheduler): correct finalizer return annotation * fix: order scheduled task runs by creation time * fix(scheduler): stabilize latest run reconciliation ordering * fix(scheduler): order latest runs by creation time * test: update trace scheduler stub * fix(scheduler): clarify reconciliation diagnostics Signed-off-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> * fix(scheduler): fail closed on startup recovery Keep single-instance parent reconciliation at startup so it cannot race manual admission. Propagate recovery failures through the Gateway lifespan before channel startup, preventing a half-started scheduler. Tests cover both recovery failure stages and a queued occurrence that survives startup before the ordinary poll drain launches it. Signed-off-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> * fix(scheduler): order occurrences and fence stale parent writes Allocate per-task occurrence sequences under the parent lock and guard parent projection across launch, recovery, completion, and queue failure paths. Track launch accounting separately so stale occurrences are counted once without replacing newer results. Commit completion and accounting atomically, preserve legacy history, and cover migrations and reordered execution on SQLite and PostgreSQL. * fix(scheduler): tighten completion projection and launch fencing diagnostics Share the once-task outcome mapping between completion and both recovery paths, validate the terminal status before opening the completion transaction, leave cron parent status untouched on completion, log the fenced launch update when an occurrence does not belong to the launched run, and drop the README capability line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(scheduler): compare caller time only against unsequenced occurrences Among sequenced rows the parent-locked occurrence_seq is the only recency key. An unsequenced row can only be legacy history or an admission by a pre-upgrade Gateway writer, so recovery prefers it over the sequence winner only when its caller timestamp is later, which is the previous ordering for that pair. A rolling upgrade therefore degrades to the pre-sequence behaviour instead of ranking every pre-upgrade admission below every sequenced one. Document that boundary instead of requiring every Gateway writer to stop before the upgrade. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(scheduler): gate once-task recovery on the same projection rule Recovery now finalises a once-task parent only from the occurrence that can_project() accepts: the highest sequenced occurrence whenever one exists, or the timestamp-latest row for a task whose history is entirely unsequenced. An unsequenced row admitted by a pre-upgrade writer can no longer cancel a parent whose sequenced occurrence is still live, nor stall finalisation of a parent whose sequenced occurrence already completed. Document that pre-upgrade instances project their own admissions during a rolling upgrade. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(scheduler): defer once-task recovery while any occurrence is live uq_scheduled_task_run_active allows one non-terminal occurrence per task, so a live row is the task's newest admission whatever its caller clock and whether it carries a sequence. Both once-task recovery paths now probe for any active occurrence after the fresh latest-run read and leave the parent untouched while one exists; cancel_stuck_once_tasks also locks the parent row so admission cannot insert a queued occurrence between that probe and the commit. Once no occurrence is live, the sequence winner decides and a terminalised unsequenced row never overrides it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(persistence): follow the local head past canonical 0019 Main's forward-revision tests assumed 0019_thread_incarnations was the local chain head. With 0022_scheduled_occurrence_seq chained after it, seed the canonical-0019 shape explicitly, assert the real head where a database is upgraded, derive the 0020 rollback binary's revision set from the ancestors of its head, and step the PostgreSQL restart scenario back to canonical 0019 before the rollback binary restarts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(migrations): describe the chain through 0022_scheduled_occurrence_seq The rolling-forward section still ended the local chain at canonical 0019; it now names 0022_scheduled_occurrence_seq as the head and lists it among the revisions the 0020 rollback-floor binary does not know. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(scheduler): accept CI's sync Postgres URL in occurrence fixtures CI hands over TEST_POSTGRES_URI as postgresql://...?sslmode=disable. The occurrence, ordering and 0022 migration fixtures built async engines from it directly, so SQLAlchemy chose psycopg2, which is not installed. Normalize the scheme to postgresql+asyncpg and drop libpq-only query keys, matching the existing 0019 migration tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(scheduler): keep the backend AGENTS.md chain within its budget The middlewares guidance chain was already above the hard limit on main, so any added byte in backend/AGENTS.md fails the agent guidance check. Leave backend/AGENTS.md identical to main and record the recovery projection rule in the 0022 migration entry, which already describes the occurrence fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Signed-off-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
Ryker_Feng
|
f52818fe5e
|
feat(skills): export custom skill packages with revision-bound preview (#5332)
* feat(skills): export custom skill packages with revision preview * docs(gateway): keep export guidance within size budget * ci: retry checks after transient uv setup download failure * docs: focus skill export agent guidance on maintenance invariants * fix(skills): handle export disconnects and bound archive transfers * docs(gateway): remove redundant export guidance to fit merged budget * fix(skills): reset export idle deadline after transfer progress |
||
|
Hyeonsang Cho
|
556975f284
|
fix(gateway): gate github_token and disable_clarification on internal callers (#5338)
* fix(gateway): gate github_token and disable_clarification on internal callers `non_interactive` is honored only for internally-authenticated callers because it strips `ask_clarification` from the lead-agent toolset. The two sibling run-context keys reproduced that effect without the gate. `merge_run_context_overrides` forwarded `_CONTEXT_RUNTIME_ONLY_KEYS` regardless of `internal`, and `strip_internal_context_keys` scrubbed only `_CONTEXT_INTERNAL_CALLER_KEYS` -- so any session or PAT caller could set `disable_clarification` through `body.context`, or through the free-form `body.config` that `build_run_config` copies verbatim. That is not a milder flag than `non_interactive`: ClarificationMiddleware answers every clarification -- `risk_confirmation` included -- with "proceed without asking" instead of interrupting, and SandboxMiddleware reads the two keys as the same non-interactive signal. `github_token` rode the same path into `runtime.context`, where the bash tool exports it as `GH_TOKEN`/`GITHUB_TOKEN`, and a copy smuggled through `body.config['configurable']` reached the checkpoint store the context-only rule exists to avoid. Both keys are produced server-side by the channel run policies, which reach the Gateway over the internally-authenticated request channel, so gate them the same way: forward them only when `internal=True`, and scrub the union `_INTERNAL_ONLY_CONTEXT_KEYS` from both config sections for every other caller. Destination stays an orthogonal axis -- `_CONTEXT_RUNTIME_ONLY_KEYS` still land in `context` alone, never in checkpoint-persisted `configurable`. Regression coverage in tests/test_gateway_services.py pins both smuggling surfaces and replays the real start_run assembly order for a session caller and for an internal one, so the GitHub channel keeps carrying its minted token. * docs(changelog): record the internal-only run-context key gate (#5338) * docs(agents): keep the run-context note inside the AGENTS.md budgets The AG002 inherited-chain check failed at this head. The new backend section and the root scheduled-task sentence added 993 B to the root and backend guidance both the sandbox and middlewares chains inherit, pushing sandbox 6 B over the 98304 B hard limit and growing the middlewares chain, which main already exceeds by 155 B. An already-over chain is only tolerated while it does not grow, so the shared ancestors had to come back to their base size. Condensed the new material and removed prose the root file was duplicating: - The trust-boundary section keeps both gated surfaces, both helpers, the trust-vs-destination split, and the disable_clarification note in half the space. - The root scheduled-task bullet names all three internal-only keys and both smuggling surfaces while staying under its previous size. - Dropped the root `scheduler.recursion_limit` bullet, which restated backend/AGENTS.md:18 almost verbatim; its one unique fact (a YAML edit needs no Gateway restart) moved to that bullet. - Deduplicated the nginx routing sentence, which already deferred to the backend routing table, and tightened the waiver note's sequencing tail. Root and backend guidance now sit 50 B under their combined base size, so the sandbox chain returns to 97310 B and the middlewares chain no longer grows. Every file stays under its AG001 soft budget. |
||
|
wutongyuonce
|
48a8978b7b
|
feat(scheduler): add interval schedule type (#5291)
* feat(scheduler): add interval schedule type Allow scheduled tasks to fire every N seconds from last dispatch, not only wall-clock cron or a single run_at. Cadence is UTC now+N with no missed-beat catch-up, bounded by min_once_delay_seconds and 30 days. * fix(scheduler): let interval tasks create, edit, and keep next run Create/edit now keep every_seconds. Unchanged interval spec no longer resets next_run_at, including timezone-only PATCH. * fix(scheduler): keep non-minute intervals on edit Stop rounding every_seconds to whole minutes in the form. Values that are not whole minutes or hours now use a seconds unit so edit/duplicate round-trips the stored cadence instead of rewriting it and resetting next_run_at. Document that min_once_delay_seconds is also the interval floor. * fix(scheduler): clamp interval seconds to the default 60s floor The new seconds unit allowed 1–59, which the API rejects under the default min_once_delay_seconds. Clamp the form to >= 60 and show the floor next to the preview. Also mention interval in the scheduler field_doc, matching config.example.yaml. * fix(scheduler): do not clamp interval amount while typing Keystroke clamp made 90 become 9 -> 60, then 600, and backspace could not leave 60. Keep the raw field text and apply the 60s floor on blur and emit only. * test(scheduler): cover interval input editing * fix(frontend): preserve saved interval cadence until edited * style(tests): format scheduled task router tests --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
wutongyuonce
|
69f0f483eb
|
feat(scheduler): let scheduled tasks pin a custom agent (#5288)
* feat(scheduler): let scheduled tasks pin a custom agent Create and update accept optional assistant_id, defaulting to lead_agent. Custom names are normalized and must already exist for the task owner. The workspace form exposes the same choice, and duplicate copies it. Fixes #5286 * fix(scheduler): keep assistant-id PR free of interval tests Drop the six interval tests that belonged to the interval schedule PR and fail here because this tree still only accepts once/cron. Treat lead_agent case-insensitively so LEAD_AGENT / lead-agent store as the default. Omit unchanged assistant_id on edit so a deleted custom agent does not 422 unrelated PATCH (rename, reschedule). * fix(scheduler): format task page and browser tests --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hyeonsang Cho
|
8e86729aa0
|
fix(gateway): confine artifact PUT to /mnt/user-data/outputs after path resolution (#5321)
* fix(gateway): confine artifact PUT to /mnt/user-data/outputs after path resolution
The outputs-only guard on PUT /api/threads/{id}/artifacts/{path} was a
string-prefix check on the raw path. A percent-encoded `..`
(`outputs/%2e%2e/uploads/x.txt`) survives nginx's variable proxy_pass
untouched, is decoded by Starlette, passes the prefix check, and the
resolver only confines the result to `user-data/` -- so an owner could
overwrite a sibling upload or workspace file in their own thread.
Collapse dot segments before the prefix check, and re-check the resolved
host path against the resolved outputs root so a symlink planted inside
`outputs/` cannot redirect the write either. The normalized virtual path
is what the response echoes and what non-mounted sandboxes receive.
* refactor(gateway): share the outputs-confinement rule with channel attachments
Review follow-up on #5321: the "only under /mnt/user-data/outputs" rule was
implemented independently by the artifact editor and by IM-channel
attachment delivery, and the two copies had already drifted.
Move it into app/gateway/path_utils.py as normalize_outputs_virtual_path
(collapse `..` before the prefix check) and resolve_outputs_confined_path
(re-check the resolved host path against the resolved outputs root, which
also catches a symlink planted inside outputs/). PUT /artifacts and
ChannelManager._resolve_attachments both call the helper; artifact_archive
keeps its stricter ZIP-member rules layered on top.
Tests that previously stubbed resolve_thread_virtual_path for the editor now
stub resolve_outputs_confined_path, and the channel attachment tests patch
path_utils.get_paths, which the helper binds at import like the other
consumers. The confinement itself is pinned by tests/test_gateway_path_utils.py.
|
||
|
hataa
|
37b03a3811
|
fix(channels): bound Discord outbound cross-loop awaits and restart dead clients, fixes #5226 (#5227)
* fix(channels): bound Discord outbound cross-loop awaits and restart dead clients, fixes #5226 * docs: reduce inherited agent guidance size * fix(channels): stop half-started channels before discarding them _start_channel now tears the instance down (stop + untrack) whenever start() raises or the channel never reaches is_running, so an outbound listener subscribed before the transport was confirmed cannot outlive its channel. Addresses the review on #5227. * fix(channels): retain half-started channels until failed-start cleanup completes Ownership in _stop_and_discard_channel now mirrors ChannelService.stop(): the instance is dropped only after its stop() completes. A cancellation arriving mid-cleanup (or a stop() that raises) leaves it tracked, so a retried readiness attempt stops it again before replacing it and service shutdown can still reach it — untracking first orphaned resources nobody could clean up. Addresses the round-3 review on #5227. * fix(channels): defer replacement when a retained channel fails to stop The pre-retry stop in ensure_channel_ready popped unconditionally, so a retained instance whose second stop() raised was untracked with its outbound listener still subscribed — the same orphan one hop later. restart_channel (del after failed stop) and remove_channel (pop before stop) had the same shape. All three now route through _stop_and_discard_channel and decline the operation for that round when the instance is retained, so _start_channel can never overwrite a still-listening channel. Addresses the review on #5227. * fix(channels): enforce the retention guarantee inside the readiness attempt loop A failed attempt whose cleanup retained the instance used to let the next attempt (attempts=2 is the production default) construct a fresh instance and overwrite the retained one via _start_channel's unconditional assignment — orphaning the first instance's subscribed listener one hop earlier than the cross-round guard covers. The guard now lives at the mechanism: _start_channel refuses to install while the name is still tracked, and ensure_channel_ready ends the loop on retention. The shared discard helper's log message is path-neutral. Addresses the review on #5227. * fix(channels): make the retained-instance guard message path-neutral The guard can fire for any still-tracked instance, not only failed cleanup, so the message must not assume the cause. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
RongJie G
|
06c827903a
|
feat(persistence): add expand-phase thread incarnation storage (#5216)
* feat(persistence): expand thread incarnation storage Add nullable thread and MCP task incarnation columns while preserving mixed-version writes. New thread records receive stable incarnation IDs, and new task rows copy the matching owned or shared thread incarnation without changing any read, claim, session, or deletion behavior. * test(persistence): pin incarnation rollback compatibility * test(api): pin internal thread response boundary * fix(persistence): rebase incarnation rollout after projects --------- Co-authored-by: CorgiBoyG <CorgiBoyG@users.noreply.github.com> |
||
|
wutongyuonce
|
3c7d3303d3
|
feat(gateway): paginate thread run history (#5283)
* feat(gateway): paginate thread run history (#5282) GET /api/threads/{thread_id}/runs stays a bare array of the newest 100 runs so LangGraph SDK clients keep working. Add GET /runs/page with a (created_at, run_id) keyset cursor so callers can walk older history. * fix(gateway): reject one-sided run history cursors RunManager.list_by_thread now raises if only one of before_created_at or before_run_id is set, matching the HTTP 422. Document the per-page sort cost on the SQL keyset query, and add the missing CHANGELOG [#5282] link definition. * fix(gateway): round-trip run page cursors through query strings Emit next_before_created_at with a Z suffix so '+' is not decoded as a space. Accept that space, and Z, when parsing. Treat blank cursor fields as absent and reject a non-ISO before_created_at in RunManager so a harness caller cannot silently restart at the newest page. * style(gateway): ruff-format run page cursor files Collapse the one-sided cursor ValueError and the two before_created_at asserts so ruff format --check passes at line-length 240. |
||
|
Zeren Wang
|
5951c89b5b
|
feat(projects): project workspaces with scoped chats and thread membership (#5265)
* feat(projects): project workspaces with scoped chats and thread membership
Backend:
- projects table model and migration; fail-closed ProjectRepository with
ownership checks, CRUD/archive/restore/delete router, and atomic thread
move between projects
- threads_meta.project_id column exposed as reserved deerflow_project_id
metadata; project-aware thread create/search with pagination bounds and
membership echoed in create responses
- first-run admission assigns the project only at genuine first run, seeded
at write time and dropped when invalid; serialized against project
deletion and thread assignment
- branch creation inherits the source thread's project membership (an
archived/deleted project degrades the branch to unassigned instead of
failing the request)
Frontend:
- projects data layer, thread move API, and sidebar projects section with
flat/grouped modes, archived-project threads, and stable virtual-list
offsets
- project detail page with project-scoped new chat
(/workspace/chats/new?project=) and paginated thread list
- move-to-project thread menu, new-project dialog, archived-project gates
- project-scoped new chats pre-create the thread with membership before the
first submit or /goal set, so runs never proceed outside the project
- goal-set preparation is fenced against conversation switches: a stale
continuation is dropped instead of saving the goal or launching the
abandoned submission on the newly opened conversation
- project thread lists join thread lifecycle invalidations (stop, pin) so
an open project page never keeps stale titles, recency, or pagination
* fix(chats): keep archive undo toast when the sidebar row unmounts
The archive success toast was fired from per-mutate callbacks passed to
mutation.mutate. React Query drops those handlers when the observer
component unmounts before the mutation settles; archiving the open chat
removes its sidebar row mid-flight, so the undo toast never appeared and
the e2e archive-undo test timed out waiting for it.
Move the success/error handlers to the mutation level (useArchiveThread
options, same pattern as useMoveThreadToProject) where callbacks are
delivered even after the originating row unmounts.
* fix(projects): pin project thread listing contract and exclude archived chats
GET /api/projects/{id}/threads returned the thread store row verbatim
(list[dict], no response_model): user_id/assistant_id leaked, any future
ThreadMetaRow column would auto-leak, and the OpenAPI schema was empty.
Return a narrow ProjectThreadResponse (the exact fields ProjectThread
declares) with the same metadata secret redaction the surrounding thread
endpoints get from _MetadataRedactingResponse.
The listing also ran search() without the archived filter, so a retired
chat rendered as a normal row on the project page while the sidebar hid
it. Search archived=False to mirror the sidebar's archived:false lists;
restore stays on the global Archived tab.
Both regressions pinned by new router tests: wire-shape allowlist and
archived-member exclusion.
* docs(migrations): record the 0019/0020 chain against the bootstrap reservation
The tree now chains 0018 -> 0019_projects -> 0020_threads_meta_project_id,
so migrations/AGENTS.md was stale twice over: the revision index stopped at
0018 and the rolling-forward section still claimed the tree 'deliberately
remains at 0018'.
Document the new head and record the intentional numeric-prefix reuse of
0019: 0019_projects is in-chain while 0019_thread_incarnations stays the
reserved, allowlisted out-of-tree rollout id. The owning rollout revision
must re-parent onto this tree's head when it merges so alembic never sees
two heads off 0018; bootstrap.py now cross-references that note next to
_FORWARD_COMPATIBLE_REVISION.
* fix(chats): invalidate project thread lists on archive/restore
useArchiveThread refreshed the infinite sidebar cache, threads/search and
the per-thread metadata cache but not the project-scoped list
([...PROJECTS_QUERY_KEY, 'threads', id]) this PR adds — the one thread
mutation not wired to that key, after usePinThread, useRenameThread,
useDeleteThread, useMoveThreadToProject and invalidateStoppedThreadCaches.
An archive from a sidebar row while a project page is open therefore left
the archived chat rendered as a normal row until remount (and undo left it
missing). Invalidate the prefix in the mutation-level success handler.
Regression test asserts the project-list prefix is invalidated on success.
* fix(projects): fetch project discovery only in grouped sidebar mode
RecentChatList mounted two useProjects queries per sidebar render, but
knownProjectIds is consumed only by the grouped-mode exclusion filter; in
the default flat mode every page load paid two GET /api/projects?status=
round trips for data nothing read. Gate both queries on grouped mode —
GroupedProjectList fetches the same keys when the toggle is on and
TanStack dedupes the observers.
Also set retry: false on useProject: a deleted or foreign project 404s
deterministically, and the page renders a dedicated not-found state for
it, so the default 1s/2s/4s retry backoff kept deep links in 'loading'
for ~7s before that state appeared. Matches useThreadMetadata /
useThreadTokenUsage.
* fix(threads): fail closed on project-scoped create in memory mode
MemoryThreadMetaStore.create accepted project_id and silently ignored it,
making memory mode the one membership path that fails open: POST
/api/threads with a project id returned 200 and the run started
unassigned, violating the invariant that a run never proceeds outside the
selected project (the SQL store raises ProjectNotAssignableError inside
the insert transaction for the same request).
Raise ProjectNotAssignableError whenever project_id is present so the
router's existing 404 mapping applies, the frontend keeps the composer
text for a retry, and memory mode behaves exactly like SQL mode.
set_project already reports rejection; create now matches it.
Store-level test (raises, nothing persisted, project filter stays empty,
unscoped creates still work) plus a router-level test asserting the 404
and that no row is left behind.
* fix(projects): window the project page thread list
ProjectThreadsSection rendered every loaded page as a plain Link row, so a
long-lived project accumulated unbounded DOM on the page's scroll surface:
each load-more appended another 100 rows and every formatTimeAgo tick
re-rendered the whole list.
Reuse VirtualThreadList (now generic over any row shape with a
thread_id), pointing its scroll parent at this page's ScrollArea viewport
via the shared [data-slot="scroll-area-viewport"] selector used by
/workspace/chats; under the 60-row threshold it falls back to the plain
render, so small projects are unchanged.
* fix(projects): restore row dividers and pin them with a render test
The row class template literal concatenated transition-colors directly
with the conditional border-b token, so non-final rows rendered the
invalid class 'transition-colorsborder-b' and lost both the divider and
the transition. Compose the row classes with cn() and a boolean guard
instead.
The section moved out of page.tsx into a testable component so the row
markup finally has coverage: a DOM test asserts every row except the
final data row carries border-b (index-based, not last: — correct under
virtualization where the last mounted row is not the last data row), and
the untitled fallback plus load-more button render for a partial page.
* fix(projects): validate forward schemas and fence membership reads
|
||
|
hataa
|
c55f242451
|
feat(authz): surface effective route permissions on GET /auth/me (Phase 4, #4063) (#5228)
* feat(authz): surface effective route permissions on GET /auth/me (Phase 4, #4063) GET /api/v1/auth/me now returns the effective route permissions alongside the user identity, so the frontend can hide actions the caller's role cannot perform (RFC #4063 Phase 4). The value reuses the AuthContext that AuthMiddleware already resolves per request (including PAT-scope intersection and internal-caller semantics), so /me adds zero extra provider evaluations; a middleware-less composition falls back to the same resolution _authenticate uses. Credential-creation responses (register/initialize) leave the field None: they are public paths where the middleware does not run, and resolving there would introduce fresh on-loop config loads on those routes. * test(e2e): expect /auth/me permissions in auth-disabled contract PR #5228 adds the effective route permissions to GET /auth/me, so the strict toEqual against the bare AUTH_DISABLED_USER object no longer holds: the received payload carries six extra keys (the permissions array). Extend the expected payload with the full registered permission set in _ALL_PERMISSIONS order — with authorization disabled the gateway grants exactly that static list, so the pin stays deterministic. The runtime frontend is unaffected (auth-disabled SSR never calls /me, and userSchema strips unknown keys); only this contract pin needed the new field. * refactor(authz): public resolve_route_permissions_for_request wrapper Address review nits on the middleware-less fallback: the router reached into the private authz._is_internal_caller, so expose a thin public wrapper pairing resolve_route_permissions with the internal-caller heuristics, and use it from both _authenticate and the /me fallback so the two cannot drift apart. Also drop an unused tmp_path parameter from test_auth_disabled_me_includes_default_admin_permissions (_setup_auth provisions its own tmp directory). No behavior change: the wrapper delegates to the exact pair of calls the fallback made before. |
||
|
wutongyuonce
|
9ad79baf97
|
feat(gateway): support idempotent thread runs (#5258)
* feat(gateway): support idempotent thread runs Accept Idempotency-Key on thread-scoped create, stream, and wait endpoints, scoped by owner and thread before durable admission.\n\nRefs #5257. * fix(gateway): handle idempotent run reuse on wait and stream Reused store-only records have no local task. /wait now waits on the bridge when it can observe the stream, and otherwise returns durable status instead of a stale checkpoint. A reused terminal stream that has been evicted emits gap/reload_durable_state. Replay is bound to the original input and assistant_id. * fix(gateway): 409 reused in-flight streams on this worker A store-only running record on a process-local bridge has no owner stream. POST /runs/stream used to subscribe anyway, which created an empty log and waited forever. Match join: 409 unless the run is already terminal, so missing-stream retries can still emit gap. * fix(gateway): keep observer joins off the idempotent stream-gap path sse_consumer keyed missing-stream gap on the sticky idempotency_reused flag, so a later join of a terminal run inherited it. Gate that branch on apply_on_disconnect, which already separates creating streams from joins. Document the retry outcomes clients have to handle. * fix(gateway): gate missing-stream gap on creating retry Reuse apply_on_disconnect to pick gap vs end changed sse_consumer default path, so a missing stream started returning gap for default callers and for out-of-scope POST /api/runs/stream. Keep that branch behind emit_gap_on_missing_stream and pass it only from thread-scoped /runs/stream on this request reuse. * fix(gateway): keep wait reuse off later checkpoints Direct handler calls were crashing because FastAPI Header() leaked in as the Python default. Bind Idempotency-Key with Annotated so the default is None, and ignore non-str keys. A reused completed /wait was still reading the latest thread checkpoint. After a later run on the same thread that is the later run's result. Return durable status instead of claiming the head as this run's output. * fix(gateway): snapshot wait reuse and refresh store status idempotency_reused lives on the shared cached record. Capture it before awaiting completion so an overlapping retry cannot suppress the original creating /wait checkpoint. A store-only peer record still holds admission-time status after the owner publishes END. Refresh durable status/error before returning them. |
||
|
Ryker_Feng
|
98b8e4657e
|
feat(chats): add archive and restore (#5236)
* feat(chats): add archive and restore * test(chats): observe archive search requests in pagination e2e * docs(gateway): move thread lifecycle details out of inherited guidance * docs(chats): add concise archive and restore RFC * docs(chats): move archive RFC discussion to issue 5237 |
||
|
PeaceMaker-best
|
e3df6ea4a8
|
feat(channels): select custom agents per conversation (#5168)
* feat(channels): select custom agents per conversation Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> * fix(channels): reserve agent slash command across clients Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> * fix(tui): hide reserved slash commands from skills Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> * fix(channels): preserve selected agent across clients --------- Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> |
||
|
Michael
|
4791e94a73
|
feat(gateway): add /health/ready readiness probe backed by the database (#5166)
* feat(gateway): add /health/ready readiness probe backed by the database
## Why
GET /health only proves the process is up: it returns 200 even when the persistence engine cannot reach the database. Orchestrators already treat it as a readiness gate (docker-compose.yaml marks the gateway service healthy and nginx depends_on service_healthy), so a DB outage or a still-migrating Postgres leaves the stack 'healthy' while every request fails.
## What changed
- New GET /health/ready endpoint: bounded SELECT 1 against the existing persistence engine (deerflow.persistence.engine.get_engine) with a 2s timeout.
- Response is 200 {'status': 'ready', 'database': 'ok'} when reachable, 503 {'status': 'degraded', 'database': 'unreachable'} when the probe fails, and 200 ready with database=not_configured for backend=memory (nothing to probe).
- GET /health is unchanged (pure liveness), and /health/ready is public through the existing /health auth whitelist.
- docker-compose.yaml gateway healthcheck now polls /health/ready so service_healthy reflects database reachability.
- Documented both endpoints in backend/app/gateway/AGENTS.md.
## Surface area
- [x] Backend API - new GET /health/ready endpoint under backend/app/gateway
- [x] Sandbox / Docker - gateway healthcheck in docker/docker-compose.yaml now gates on readiness
- [ ] Frontend UI / Agents / Skills / Dependencies
- [x] Default behavior change - existing /health unchanged; the prod compose healthcheck is stricter (503 while the database is unreachable)
## Validation
- New unit tests in backend/tests/test_gateway_health.py cover ok / unreachable / not_configured probe results and the 200/503 payload mapping (6 passed).
- app.gateway.app imports cleanly and registers both /health and /health/ready.
- ruff check + ruff format clean.
## AI assistance
**Tool(s) used:** Codex (coding agent)
**How you used it:** design, implementation, and unit tests produced with AI assistance; reviewed before commit.
- [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.
* fix(helm): point the gateway readiness probe at /health/ready
## Why
Review on #5166 (willem-bd, P1): the chart still probed /health for readiness,
so Kubernetes marked the pod ready and routed traffic while the database was
unreachable - exactly the failure mode /health/ready was added to catch.
## What changed
- deploy/helm/deer-flow/templates/gateway-deployment.yaml: readinessProbe
httpGet.path now hits /health/ready (DB-backed, 503 while the database is
unreachable). The liveness probe stays on /health.
## Verification
- One-line path change inside the existing readinessProbe block; git diff
confirms only the readiness path changed (liveness untouched).
## AI assistance
**Tool(s) used:** Codex (coding agent)
**How you used it:** implemented the reviewer-requested probe path 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.
* fix(gateway): readiness probe also checks the effective checkpointer/Store backend
## Why
Follow-up review on #5166 (willem-bd, P1): get_engine() only represents the
ORM backend selected by `database:`. The legacy `checkpointer:` section takes
precedence for the LangGraph checkpointer and Store, so a split configuration
(a local SQLite/memory `database:` with `checkpointer.type: postgres`) could
report 200 while the PostgreSQL backend agent runs depend on was down.
## What changed
- GET /health/ready now probes both persistence halves: the ORM engine behind
`database:` (unchanged) and the effective LangGraph checkpointer/Store
backend resolved with the runtime's own rule (legacy `checkpointer:` config,
otherwise derived from `database:`), for memory/sqlite/postgres backends.
- The payload gains a `checkpointer` field with the same
ok / not_configured / unreachable vocabulary as `database`; 503 degraded is
returned when either probe is unreachable.
- Probes are bounded by the existing 2s timeout: sqlite via aiosqlite SELECT 1
on the resolved path, postgres via a bounded psycopg AsyncConnection SELECT 1
on the DSN with the configured search_path. A missing driver for a configured
backend degrades readiness (the runtime could not run either).
- Documented the two-probe semantics in the endpoint docstring and
backend/app/gateway/AGENTS.md.
## Verification
- New tests: healthy ORM engine + unreachable legacy checkpointer backend ->
503 degraded with database: ok / checkpointer: unreachable; checkpointer
probe mapping for memory/sqlite(postgres missing-driver) backends; existing
payload tests now pin the checkpointer field.
- cd backend && python -m pytest tests/test_gateway_health.py: 11 passed.
- app.gateway.app imports cleanly; ruff check + ruff format clean.
## AI assistance
**Tool(s) used:** Codex (coding agent)
**How you used it:** design, implementation, and unit tests produced with AI assistance; reviewed before commit.
- [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.
* fix(gateway): bound /health/ready to one deadline and probe the startup checkpointer snapshot
## Why
Second round of review on #5166 (zhfeng P1/P2, willem-bd P1/P1/P2). Three
correctness issues remained in the readiness endpoint:
- The database and checkpointer probes ran sequentially, each allowed 2s, so a
healthy response could take almost 4s - past Kubernetes' 1s default
readinessProbe timeout and inside Docker's 3s client timeout. A slow but
healthy backend could make every replica unready.
- The checkpointer probe re-resolved process-wide, hot-reloaded configuration
per request, while app.state.checkpointer/store are built once from the
startup_config snapshot in langgraph_runtime(). After a live config edit the
endpoint could probe a backend the running gateway does not use, and a
resolution failure was swallowed into None -> not_configured -> 200.
- The SQLite probe opened the path with aiosqlite.connect(), which creates the
file when missing: a deleted checkpoint database was silently resurrected as
an empty file and reported ok instead of surfacing the outage.
## What changed
- backend/app/gateway/health.py: the two probes now run concurrently beneath a
single endpoint-wide deadline (_READINESS_DEADLINE_SECONDS=3.0) so a healthy
response completes within one probe window (~2s), never the sum of both.
A probe that overruns the deadline degrades the endpoint instead of hanging.
- langgraph_runtime() now records the checkpointer/Store config resolved from
the same startup_config snapshot its checkpointer/store singletons are built
from (app.state.checkpointer_config); /health/ready probes that snapshot and
never re-resolves hot-reloaded config. resolve_checkpointer_config() returns
None on resolution failure and the endpoint fails closed (503, checkpointer:
unreachable) instead of reporting not_configured.
- The SQLite probe opens disk-backed paths with the non-creating mode=rw URI
flag, so a missing database file stays missing and yields unreachable;
in-memory forms (:memory:, file:...mode=memory) have nothing external to
probe and report not_configured like the memory backend.
- Orchestrator timeouts now sit above the endpoint bound: Helm readinessProbe
gains timeoutSeconds: 5 (Kubernetes default is 1s) and the docker-compose
gateway healthcheck client timeout moves from 3s to 5s.
## Verification
- New regression tests: concurrent probes keep total elapsed time within one
probe window; a probe ignoring its budget trips the endpoint deadline to 503;
missing SQLite file stays absent and yields unreachable; in-memory SQLite
forms map to not_configured; missing startup snapshot / config resolution
failure fail closed to 503; resolve_checkpointer_config() raising is covered.
- cd backend && python -m pytest tests/test_gateway_health.py: 21 passed;
tests/test_gateway_docs_toggle.py and lifespan/shutdown gateway suites pass.
- ruff check + ruff format clean on all changed files.
## AI assistance
**Tool(s) used:** Codex (coding agent)
**How you used it:** implemented the reviewer-requested concurrency/deadline, startup-snapshot probing, fail-closed resolution, and non-creating SQLite probe; reviewed before commit.
- [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.
* fix(gateway): serialize connection-opening readiness probes behind a strict gate
## Why
Review on #5166 (willem-bd, P1): every request to /health/ready opened a new
PostgreSQL connection in _probe_postgres_backend, outside both the ORM pool and
the runtime checkpointer pool. The route is public through the /health auth
prefix and nginx proxies /health/*, so concurrent unauthenticated requests
could create an unbounded number of connections (each held for up to two
seconds), exhaust PostgreSQL max_connections, and take down both normal
traffic and the readiness probe itself.
## What changed
- backend/app/gateway/health.py: connection-opening checkpointer probes
(sqlite connect, postgres AsyncConnection.connect) now run inside a strict
per-process gate - an asyncio.Lock cached per running event loop - so at
most one probe connection can be in flight per worker process. Requests
that queue behind the gate are still shed by the existing endpoint-wide
deadline, so a flood cannot pile up new connections or open files.
- Memory and unknown-backend decisions stay outside the gate; payload and
probe semantics are unchanged. The serialization is documented in the
module docstring and backend/app/gateway/AGENTS.md.
## Verification
- New regression test: 8 concurrent readiness_payload() requests against an
instrumented sqlite probe assert the maximum number of in-flight probe
connections is 1 while every request still returns 200.
- cd backend && python -m pytest tests/test_gateway_health.py: 22 passed;
tests/test_gateway_docs_toggle.py and tests/test_gateway_lifespan_shutdown.py
also pass on the merged main head.
- ruff check + ruff format clean on all changed files.
## AI assistance
**Tool(s) used:** Codex (coding agent)
**How you used it:** implemented the reviewer-requested strict concurrency bound for the public readiness probe; reviewed before commit.
- [ ] I've read and understand every line of this change and take responsibility for it — it's not unreviewed AI output.
|
||
|
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>
|
||
|
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> |
||
|
Aari
|
9e0fbd60fa
|
fix(sandbox): isolate concurrent subagent shell sessions (#5134)
* fix(sandbox): isolate concurrent subagent shell sessions * fix(sandbox): make execution acquire idempotent * fix(sandbox): close execution lifecycle gaps * fix(sandbox): serialize retained client lifecycle * fix(sandbox): close remaining client lifecycle gaps * fix(sandbox): unwind failed client lookup * fix(sandbox): protect internal lease identities * fix(sandbox): make cancellation reconciliation durable * fix(sandbox): fence cancelled workers and IM uploads |
||
|
hataa
|
08b27aef73
|
feat(auth): make login rate-limit parameters configurable, fixes #5108 (#5110)
* feat(auth): make login rate-limit parameters configurable, fixes #5108 Add auth.local.max_login_attempts (default 5) and auth.local.lockout_seconds (default 300) so operators can tune the per-IP login throttle: raise the ceiling for shared-egress-IP offices behind proxies/NAT, or tighten it for stricter posture. Policy is live-read per call (matching the _local_registration_enabled precedent), so a config reload applies without a Gateway restart; raising the threshold mid-lockout immediately unblocks affected IPs. Review feedback addressed (willem-bd): - Only FileNotFoundError falls back to the hardcoded defaults; a malformed config propagates, mirroring _local_registration_enabled, so an operator who tightened the policy never silently gets the more permissive defaults. - _check_rate_limit looks up the record before resolving the policy, so a clean IP pays zero config reads (get_app_config re-hashes config.yaml per call and login_local is an unauthenticated async endpoint). Bumps config_version to 39 in config.example.yaml and the Helm chart (values.yaml + README example) so the chart drift check stays green. * fix(auth): reject max_login_attempts=1 and honor live lockout_seconds for active lockouts * fix(auth): close live-policy state gaps in login throttle (resurrection, count reset, broken-config verification) * fix(auth): commit evaluated lockout duration on decreases too, preventing raise-resurrection * test(auth): pin broken-config fail-closed sequence through the login route * fix(auth): sweep expired locks by stored sentence and keep policy reads off the event loop * fix(auth): re-read throttle record after the policy-resolution yield point --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
wutongyuonce
|
fb722770e4
|
fix(agents): do not hide invalid config with file fallback (#4952)
* fix(agents): do not hide invalid config with file fallback * test(agents): cover invalid on-disk config fallback * fix(agents): resolve stores off the event loop * fix(agents): distinguish missing nested config from main config * fix(agents): reject missing explicit config path * test(agents): isolate config fallback test * test(agents): isolate router blocking IO coverage * test(agents): pin malformed config.yaml parse-error propagation An unparseable config.yaml used to be swallowed by the broad except Exception and silently downgrade to FileAgentStore. The narrowed except FileNotFoundError already propagates yaml.ParserError/ScannerError; pin that contract with a real on-disk config instead of monkeypatched get_app_config. |