* fix(middlewares): end length-capped turns cleanly, prevent todo re-engagement, annotate write_file budget
When a model hits its per-response output cap (finish_reason=length) while
emitting a write_file tool call, ModelLengthFinishReasonMiddleware suppresses
the truncated call and stamps model_length_termination. TodoMiddleware must
not re-engage (jump_to=model) on such a capped turn -- doing so re-emits the
same oversized call into the same cap, producing up to 3 futile responses
with junk fragments instead of a clean truncation notice.
Changes:
- TodoMiddleware.after_model: skip completion reminder jump when
additional_kwargs.model_length_termination is present (follows the existing
deerflow_error_fallback precedent).
- ModelLengthFinishReasonMiddleware: always append the length notice when
tool calls were suppressed, even when partial text survived (collapses the
visible-content ternary). Fixes a latent bug in append_visible_text that
silently dropped string content.
- tools.get_available_tools: annotate write_file's model-visible description
with the model's configured max_tokens output budget. Guarded extraction
safely handles missing or non-numeric tokens, and the tool is cloned via
model_copy to keep module-level singletons immutable across assemblies and
prevent guidance leakage to unbudgeted models.
- release_policy_parameters() updated for both middlewares.
- AGENTS.md chain entries (#20, #35) and module docstrings updated within
AG002 guidance limits.
- Tests: 8 new/focused unit tests + 1 updated pin + 1 real create_agent()
integration test reproducing the incident (thread b1723286).
* fix(tools): use effective model cap for write_file guidance
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(sandbox): stop E2B reconciliation from reviving warm-pool sandboxes
Periodic reconciliation probed every discovered remote sandbox with
Sandbox.connect() before the locality check, and the check itself only
consulted _sandboxes, not _warm_pool. A sandbox parked by release() was
therefore adopted back to active on the first pass, and because the SDK
normalizes connect(timeout=None) to its 300s default and the control
plane extends a running sandbox's expiry when now+timeout is later,
each 60s pass kept pushing the expiry forward — idle warm sandboxes
never hit their configured idle_timeout.
Treat _sandboxes and _warm_pool ids as locally tracked up front: skip
probing them (no timeout-mutating connect), keep them canonical, and
route only genuinely remote candidates through the duplicate-reap path.
Extend the post-probe adoption recheck to _warm_pool so a release that
lands mid-probe cannot be promoted back to active either.
Fixes#5550
* fix(sandbox): keep active E2B VMs alive and sweep expired warm entries
Address review on #5562:
- Reconciliation now refreshes the remote TTL of locally active
sandboxes through their cached client (never connect()), restoring
the keepalive for turns that outlive idle_timeout without reviving
warm-pool VMs.
- Warm-pool entries parked longer than idle_timeout are dropped during
reconciliation — their VMs are expected to be reaped by the control
plane — releasing the ownership lease and the capacity slot they
would otherwise pin until reclaim, eviction, or shutdown.
- Remove the now-dead thread-local canonical sort; locally tracked ids
are skipped unconditionally, so the ordering hint had no effect.
* fix(sandbox): preserve active E2B keepalive and shared capacity
* fix(sandbox): serialize E2B reconciliation lifecycle transitions
* fix(sandbox): fence E2B ownership and timeout lifecycle writes
* fix(sandbox): isolate ownership heartbeats from E2B timeout IO
---------
Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(mcp): re-scope to MCP task claim lifecycle only
Keep PR #4966 a small, closed MCP lease/cancellation state-machine change and
move RunJournal and Run lifecycle work into dedicated follow-ups. This branch
contains only the MCP task claim lifecycle:
- mcp task release/snapshot fencing by owner + per-claim lease token
- phase-level single-flight poll/cancel/notification owners with retained handoff
- routine cancellation no longer persisted as a task failure diagnostic
- bounded ordinary release ownership retention past the drain deadline
- 0018_mcp_task_lease_tokens migration + migration/bootstrap head assertions
- wait_for_task_until helper (MCP uses it); worker-specific capture helper moved
to the run-finalization follow-up
RunJournal (journal.py + test_run_journal.py) and run lifecycle
(manager/worker/store/run sql + run tests) are preserved on
backup/cancellation-safety-full and will be raised as separate follow-ups.
* fix(mcp): unblock claims after ambiguous handoff resolves
A phase-level single-flight owner only guards an ambiguous claim outcome. Once
the claim resolves, the phase owner is released immediately; the handoff may
continue releasing returned rows as bounded, service-owned background work
(transferred to _compensation_tasks on timeout). Per-claim token fencing rejects
a late release against a newer claim generation, so a stuck release no longer
locks the whole phase until process restart.
- README: drop the stale progress-snapshot sentence from the bounded ordinary
release description.
- service: pop the identity-checked phase owner as soon as the claim outcome is
known, then release returned rows with the bounded path; carry the release in
_compensation_tasks if it exceeds the drain deadline.
- mcp/AGENTS.md: document that only an unresolved claim outcome (not the handoff)
blocks later phase scans, and that returned-row releases may continue in the
background once the owner is released.
- tests: pin that the phase owner is released before a stuck release finishes
while the release stays service strong-owned.
* refactor(mcp): remove unused single-record claim wrappers
_poll_one, _cancel_one, and _notify_one are unreachable in production: the
worker always processes claimed records through _run_claimed_batch, so these
wrappers preserved a second, dead single-record lifecycle (state is None)
whose only observable behavior was a wrapper-specific cancellation release.
Remove the three wrappers and migrate the regressions that guarded their
cancel/release invariants to exercise the production _run_claimed_batch path
(operation=_*_one_claimed, release=_release_*_after_cancellation). The single
wrapper-only "state is None" contract (test_poll_release_hang_without_batch)
is deleted; all 11 remaining invariants (CancelledError preservation, repeated
cancellation, poll-only token-fenced lease release, notification claimed vs
dispatched phase release, hung compensation -> service ownership, and
background compensation exactly-once observation) are now covered through the
real batch lifecycle.
* fix(mcp): fence claim-owned mutations against stale generations
The per-claim token check in the ORM release/apply paths was only in the
SELECT; the final write went out by primary key. On SQLite (where
with_for_update() is a no-op) a mutation from an older claim generation
could therefore clear a claim that a newer generation had reclaimed after lease
expiry — the exact distributed lease-fencing failure the per-claim token was
meant to prevent.
Make every claim-owned mutation a single atomic conditional UPDATE with the
owner and per-claim token in the WHERE clause (rowcount 0 => stale, return
False, no mutation):
- release_claim: atomic fence; record the poll-failure event after the fence
wins (same transaction, holding the write lock).
- apply_snapshot / apply_cancel_snapshot: atomic fence; record the event after.
- finish_notification_run: atomic fence; use a CASE on event_version >>
dispatch_version to keep a newer event pending for redelivery instead of
swallowing it as delivered.
Add one regression per path: a stale generation's release/apply/finish after a
same-worker reclaim is rejected and never clears the newer claim.
* test(mcp): pin the migration chain head to the lease-token revision
0026_mcp_task_lease_tokens becomes the alembic head, so the chain-head pin in the 0025 repair test had to move on. Follow the 0023 precedent there (single head plus expected predecessor) instead of pinning a literal head, and give the new revision its own migration test, which owns the pin and covers the nullable claim-token columns on upgrade and their removal on downgrade.
* refactor(mcp): close cancellation cleanup leftovers
* fix(mcp): retain cancelled release diagnostics
* test(mcp): remove obsolete settled compensation case
* test(mcp): cover interleaved lease reclaim races
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
The AIO sandbox's implicit persistent shell session hangs forever when a
command containing a bare 'exit' is executed: exit kills the session's
shell process, and the server's response path for that exec_command
request never completes. Verified against a standalone all-in-one-sandbox
1.11.0 container via the raw SDK:
shell.exec_command('seq 1 1000 | head -n 5') # OK (not a SIGPIPE issue)
shell.exec_command('echo x; exit 0') # HANGS every time
shell.exec_command('echo after') # OK (server recreates shell)
shell.exec_command('( echo x; exit 0 )') # OK, exit code propagates
remote_list_dir_command and remote_search_command both end their probe
scripts with a bare 'exit' (to propagate the find/grep status code), so
every list_dir/grep/glob call deterministically wedges the session —
this is the root cause behind parallel [ls, bash] tool calls deadlocking
an entire run (same defect family as #1433 and #5128).
Fix: keep 'set +e' as the outermost prefix (pinned by existing tests) and
wrap the rest of each probe script in a subshell, so exit only terminates
the subshell. Output and exit codes propagate identically.
Tests: 116 passed (backend/tests/test_aio_sandbox.py, test_remote_list_dir.py,
test_remote_search.py); one endswith assertion updated to the wrapped form.
Co-authored-by: mad_max <mad_max@coscoshipping.local>
* feat(authz): gate composer send on runs:create (Phase 4 follow-up, #4063)
Mirrors #5294's stop gating on the send side: both chat routes pass
canCreateRuns (from PERMISSIONS.RUNS_CREATE, lockstep with the backend
enum) into the shared composer. The gate sits at the top of
submitThreadMessage — the single choke point every composer entry
(submit button, Enter, goal-set-triggered run) funnels through — and
denies with a toast plus a rejected promise so PromptInput keeps the
text. The idle submit button is disabled and explains the boundary via
conditionally-spread aria-label/title (startTurnUnavailable, en/zh);
while streaming the button stays the runs:cancel stop affordance.
Also removes the unreachable kind === "stop" branch in handleSubmit
(flagged during #5294's review): the Enter path early-returns with the
streaming toast before the classifier runs.
* fix(frontend): reject denied goal starts before saving state
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(channels): add WeChat QR login and binding recovery
* fix(channels): enforce single-worker WeChat QR login and preserve bot ID
Reject QR login endpoints when multiple Gateway workers are configured, while keeping manual token setup available.
Preserve the configured bot ID when the provider omits it or returns an empty value. Add regression coverage for worker guards and credential persistence.
* fix(channels): sync WeChat completion state on provider updates
Show the connected step when refreshed provider data confirms the binding, so the dialog no longer waits indefinitely after polling is cancelled.
Add regression tests for provider updates during pending poll and binding requests, including late responses and expiry.
* fix(channels): preserve WeChat pairing codes across waits and redirects
---------
Co-authored-by: YxinMiracle <“939157765@qq.com”>
* fix(memory): report non-mapping Honcho backend_config values as ValueError
failure_policy, workspace_overrides and user_peer_overrides were read with a
falsy-only `or {}` fallback, so a truthy non-mapping (a bare string, a YAML
list) reached .get/.items and escaped as a bare AttributeError from inside
backend construction. Route all three through one _mapping helper that keeps
falsy values meaning "unset" and names the offending key as a ValueError, the
posture the mem0 and OpenViking backends already take.
* fix(memory): name the Honcho numeric knobs that cannot be cast
timeout_seconds / connect_timeout_seconds / message_char_limit /
max_injection_chars still reached float() / int() with a YAML null or a
mapping, so the operator got a TypeError naming neither the key nor the
config file. Narrow all four through one _number helper that keeps falsy
values meaning "unset" the way the sibling api_key / storage_path scalars
already do, and reports a value that cannot be cast as a ValueError on the
key. Numeric strings keep parsing, since that is what float/int accept.
* fix(memory): report non-numeric backend_config knobs by name in mem0 and OpenViking
mem0 casts top_k, score_threshold, max_injection_chars and timeout_seconds, and
OpenViking casts timeout_seconds, max_seen_message_ids, retrieval.top_k and
retrieval.max_injection_chars, straight through int()/float(). A knob written
without a value in YAML therefore escapes as "TypeError: int() argument must be
a string..." from inside backend construction, naming neither which knob nor
which file is wrong, and a non-numeric value escapes as the equally anonymous
"could not convert string to float".
Both now resolve numeric knobs through a helper that keeps a value-less key at
its default, the way the same dicts already treat failure_policy and
allow_insecure_http, and turns an uncastable value into a ValueError that names
the knob. OpenViking's score_threshold keeps None as a meaningful value rather
than a default to fall back to.
* fix(client): stop embedded uploads from writing through symlinks
DeerFlowClient.upload_files copied each file with shutil.copy2 and let
convert_file_to_markdown write the companion straight into the uploads
directory. Local and AIO sandboxes can write to that directory, so a
symlink planted at an upload name or at the companion's name was
followed: the upload's bytes and the converted Markdown landed in
whatever host file the link pointed to, and the call reported success.
The Gateway refuses symlinked destinations and the IM channels write
through write_upload_file_no_symlink; the embedded client never adopted
either.
Uploads now go through copy_upload_file_no_symlink, a new helper next
to write_upload_file_no_symlink. It keeps copy2's content, permission
bits and timestamps, so files stay readable to Docker sandboxes, but
applies them to the descriptor opened with O_NOFOLLOW and opens the
source first so a missing source cannot truncate an existing upload.
As in the Gateway, a file with an unsafe destination is skipped and
listed in skipped_files, success turns false, and the message says how
many were skipped. The companion is converted inside a private temporary
directory and then written with write_upload_file_no_symlink; one whose
name is unsafe is left out like a failed conversion, and the original
upload is kept.
* docs(changelog): note embedded upload symlink fix (#5578)
* fix(client): keep copy2's same-file guard and companion permissions
Review follow-up. Two regressions in the previous commit.
copy_upload_file_no_symlink opened the destination before comparing it
with the source, and that open truncates. Passing a file that already
sits in the thread's uploads directory therefore copied an emptied file
over itself: the upload reported success with size 0 and the original
bytes were gone, where copy2 raised SameFileError and left the file
alone. The destination is now compared with the source through
os.path.samestat before anything is opened, so identity — including a
hardlink or another spelling of the same path — raises SameFileError as
before.
The Markdown companion was published with write_upload_file_no_symlink,
which creates a new file as 0600 and ignores the converted file's mode.
Under umask 022 the companion became 0600 while its own document stayed
0644, so a bind-mounted sandbox running as another uid could read the
upload but not the Markdown the response advertises. It now goes
through the same copy helper as the upload, which preserves the
converter's permission bits.
load_codex_cli_credential copied tokens.account_id straight into CodexCliCredential.account_id with no type check. A Codex auth file whose account_id is JSON null therefore propagated None into CodexChatModel._account_id, and model_post_init raised TypeError: 'NoneType' object is not subscriptable on the account prefix in its log line. A numeric or boolean account_id slipped through the same way, reaching the ChatGPT-Account-ID header as a non-string.
Treat any non-string account_id as absent and fall back to the credential's empty-string default, matching how a missing account_id already behaves.
* fix(gateway): classify Windows SVG MIME alias as active content
Treat Windows' image/svg alias like the standard image/svg+xml active content type.
* test(gateway): cover Windows SVG MIME alias
Pin image/svg classification independently of the host MIME database.
* docs(utils): document platform MIME aliases
Record the shared active-content classification invariant.
`_extract_claude_code_credential` copied `expiresAt` straight into
`ClaudeCodeCredential.expires_at`, so a credentials file whose `expiresAt` is a
string, null, list or object reached `is_expired` and raised
`TypeError: '<=' not supported between instances of 'str' and 'int'`. That
aborted the whole lookup instead of skipping the malformed source and moving on
down the documented order, the way the rest of the loader already behaves for a
malformed `claudeAiOauth` container.
Validate the field the way the sibling branches validate their input: log a
debug line and skip the source so the next candidate is tried.
* fix(sandbox): report an exactly-full search result as complete in the remote providers
`glob` and `grep` decide `truncated` twice: once for the raw output cap
(`parse_remote_search_output`, unchanged) and once for `max_results` after the
Python-side filters have run. The second decision returned as soon as
`max_results` matches had been collected, which cannot tell a search that held
exactly that many from one that held more — a tree holding exactly
`max_results` eligible matches came back flagged as cut off, and the tool then
told the model the result was incomplete.
These providers hold the whole listing (the raw stream is capped at
`max(max_results * 4, max_results + 50)` lines and reports its own cut-off), so
like AIO's `glob` branches they can look one match past the cap before
deciding: `AioSandbox.grep`, plus `glob`/`grep` in E2B, OpenSandbox, Tenki and
BoxLite now use the same `len(matches) > max_results` rule. This completes what
#5449 started for AIO's `glob`; the local provider's half is #5491.
Co-Authored-By: Claude Code <noreply@anthropic.com>
* fix(sandbox): let remote grep see one match past the per-file cap
E2B and OpenSandbox stopped each file's grep at max(max_results, 50)
matches, so a single file holding more than max_results hits — with a
raw stream far below its limit — ended the Python loop exactly at the
cap and reported the result as complete (#5534 review).
Retain one extra match per file so the one-match lookahead can observe
the overflow and report truncation. A single-file regression at
max_results=50 covers 50 matches (complete) vs 51 (truncated) for both
providers.
Co-Authored-By: Claude Code <noreply@anthropic.com>
---------
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(frontend): rejoin active runs after reopening chats
* test(frontend): mock thread runs query in stream options test
* fix(frontend): avoid rejoining completed runs from stale cache
* fix(frontend): tighten active run recovery cleanup
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
load_codex_cli_credential called .get on the parsed ~/.codex/auth.json
(and $CODEX_AUTH_PATH) without checking that the top level is an object.
_load_json_file returns any valid JSON value, so an array or scalar payload
raised AttributeError out of CodexChatModel.model_post_init instead of the
documented 'Codex CLI credential not found' error. Guard the top level the
same way the sibling Claude loader and its own nested tokens guard do.
* fix(ragflow): batch validation for large document selections
* docs(ragflow): align documentation language with repository conventions
* docs(ragflow): preserve spacing before validation heading
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(middleware): add deterministic PII redaction for model-bound context
* fix(middleware): claim national IDs before cards, redact Command results, preserve ToolMessage fields
- Reorder detectors so checksum-gated national IDs run before the credit-card
detector; an 18-digit resident ID whose digit run also passes Luhn is no
longer mislabeled [CREDIT_CARD_n] (review finding, reproduced at 0a2a9d0)
- Redact ToolMessages carried in Command.update.messages, mirroring
ToolResultSanitizationMiddleware's dc_replace pattern
- Rebuild redacted ToolMessages via model_copy so artifact and
response_metadata survive
- Extend the numbered middleware chain in agents/middlewares/AGENTS.md
* fix(middleware): span one redactor per Command result; refresh stale AGENTS.md entry range
- Placeholder numbering now continues across every ToolMessage carried in a
single Command result (one _Redactor per _redact_result call) instead of
restarting per message
- The renumbered AGENTS.md chain still referenced entries 9-12 in the
ToolReceiptMiddleware entry; it now reads entries 10-13
* docs(agents): trim PiiRedactionMiddleware entry to fit the AGENTS.md chain budget
The main merge (fb36e0e) pushed the effective middlewares chain to 98341
bytes, 37 over the 98304 hard limit checked by agent-guidance (AG002).
Compress the entry while keeping the load-bearing facts: config gate, both
interception points incl. Command coverage, detector order rationale,
per-result numbering continuity, irreversibility, memory follow-up.
* fix(middleware): redact compaction input and reinjected summaries; harden detectors
Review round 3 on #5527:
- [P1] SummarizationMiddleware invokes its summary model directly from
before_model, outside PiiRedactionMiddleware's wrap_model_call, so raw
thread state reached the summary model and reinjected summaries carried
raw PII into model-bound context. Add a shared redact_text() seam: the
compaction prompt is redacted in _build_summary_prompt (app_config
already flows into the middleware) and DurableContextMiddleware redacts
summary_text at reinjection via a new pii_redaction_config knob wired
at both assembly sites.
- [P2] CUIT is 2+8+1 digits, not 2+10+1.
- [P2] Digit-anchored patterns use digit-aware lookarounds instead of
Unicode \b, which CJK characters defeat (身份证110105… / 手机号138…).
- [P2] The international-phone pattern no longer treats newlines as
separators, so a candidate cannot swallow the following numeric field
and then fail validation as a whole.
* fix(pii): redact title input and reserve summary placeholders
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(docker): add 'make prod-logs' entry point and hint when dev logs are empty
`make up` starts the production stack (deploy.sh: project `deer-flow`,
docker/docker-compose.yaml) while `make docker-logs` tails the dev stack
(project `deer-flow-dev`, docker-compose-dev.yaml), so after `make up`
it printed nothing (#5529), and no make entry point showed production
logs at all.
- scripts/docker.sh logs gains `--prod`: targets the stack deploy.sh
started, passes --env-file ../.env when present, and exports the same
interpolation defaults deploy.sh exports before every compose call —
without them the production volume specs fail to parse on checkouts
without .env.
- dev-only `logs` with no running containers now prints a hint pointing
at `make prod-logs` instead of staying silent.
- Makefile gains `prod-logs`, listed under Docker Production Commands.
New tests cover production targeting and the empty-state hint; they are
red on unfixed main and green here. Verified live: with a deer-flow
redis running, `logs --prod --redis` streams its logs.
Fixes#5529
* fix(docker): append --env-file after compose detection rebuilds COMPOSE_CMD
compose_preflight() probes the Compose binary and rebuilds COMPOSE_CMD,
so an --env-file appended before it was silently dropped. Append it
after preflight instead, and drive the regression test through the real
detection path (stub Docker Compose version v5.3.1, not require_compose_version)
so the append cannot regress silently.
Reviewed-in: #5538
* fix(utils): return empty text for content-less messages
message_content_to_text fell through to str(content), so a message whose
content is None yielded the truthy literal "None". Two call sites already
work around it with a local `or ""` and name the helper in the comment; the
subagent executor's `text if text else "No response generated"` fallback and
the archive's `if not text: continue` skip cannot work around it, so a
content-less terminal turn was reported as an answer of "None" and a
content-less LLM error fallback surfaced "None" instead of its error_detail.
* test(utils): cover non-None content compatibility and document fallbacks
* test(utils): cover contentless task history and refresh guard comments
---------
Co-authored-by: Lengshuang <90967079+Lesereingrape@users.noreply.github.com>
Co-authored-by: JasonH <4430962+yang0228@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(agents): close delegations a stopped run left in progress
Every task call is recorded in the delegation ledger as in_progress and
only moves on when its ToolMessage arrives. When the user stops a run
while a subagent is running, the task tool re-raises the cancellation
and no ToolMessage is written, so the entry stayed in_progress for the
rest of the thread and every later model call was told "already
delegated; do NOT delegate again; wait for or build on the result".
When a run starts with a new user message, mark entries that an earlier
run left in_progress and that have no ToolMessage as cancelled. Resumed
runs, which have no new user message, keep the current behaviour.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(agents): share the run-opening boundary between capture and closure
Also pin that in_progress entries without a run_id are never closed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(agents): clarify legacy delegation reply handling
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat(knowledge): add verifiable RAGFlow source citations
* docs(knowledge): scope RAGFlow guidance to its own directory
* fix(knowledge): preserve citations through rendering and budgets
* fix(uploads): delete the requested upload, not a symlink's target
delete_file_safe resolved the requested path before unlinking it. The
uploads directory is writable from local and AIO sandboxes, so a
symlink planted under an upload name was followed: deleting alias.pdf
removed the victim.pdf it pointed to, and the companion cleanup then
removed victim.md, while the link itself survived and the call reported
"Deleted alias.pdf". A link resolving outside the directory was already
refused by the traversal check, so the damage stayed inside the
thread's uploads.
The function now checks and unlinks the requested entry itself and
treats a symlink as not found, the same way list_files_in_dir already
hides it. unlink() never follows the final component, so a file swapped
for a link between the check and the unlink removes only the link.
Tests cover the helper, the Gateway DELETE route, and
DeerFlowClient.delete_upload.
* docs(changelog): note upload delete symlink fix (#5547)
* fix(events): serialize DB deletion with thread writers
* fix(runs): delete thread history without dropping reservations
* fix(feedback): support owner-scoped thread cleanup
* fix(threads): clean persisted records on deletion
* fix(threads): correct the feedback cleanup rationale
* test(runs): drop the wall-clock probe from the in-flight delete test
* docs: record the thread-delete and event-store fence contracts
* fix(threads): preserve legacy event-store delete compatibility
* fix(persistence): repair run-change clock schema skipped by the 0023 insertion
0023_run_change_seq was chained ahead of the already-shipped
0023_user_preferences revision, so databases stamped at that revision or
later treat it as an applied ancestor and never execute it: the
run_change_clock table and runs.change_seq column are permanently missing
and the first thread deletion fails with 'no such table:
run_change_clock' (#5516). 0025_repair_run_change_seq re-applies the same
guarded DDL on upgrade and no-ops on healthy shapes. RunChangeClockRow and
UserPreferenceRow are also registered in the ORM model registry.
Fixes#5516
* fix(persistence): preserve run-change schema when rolling back repair
---------
Co-authored-by: 1553126902 <1553126902@qq.com>
* feat(gateway): thread checkpoint retention service on the #4189 deletion contract
Implements exactly the two contract-proven deletion shapes (trailing
duration-only leaves, opt-in leaf sibling branches) with head-chain
protection, explicit id protection, a strict pending-writes guard, and
joint writes-row cleanup. Head resolution uses LangGraph's time-ordered
checkpoint ids; storage deletion mirrors the contract's per-backend data
model. Ships without a production trigger by design. Validated against
the contract suite (12 passed) plus 14 service scenarios across memory
and SQLite; Postgres paths are gated on TEST_POSTGRES_URI.
Signed-off-by: zengbohan1 <310902929+zengbohan1@users.noreply.github.com>
* fix(gateway): survivor-reachability blob GC and memory blob stats in retention service
Aligns the deletion service with the review-hardened contract: blob rows
are garbage-collected in a whole-thread pass against surviving
checkpoints' channel_versions (a real duration-only leaf shares its
parent's versions, so per-checkpoint version deletion would corrupt the
surviving state), the memory branch of the stats helper counts
saver.blobs and returns the full normalized shape, and per-node channel
versions are collected during the graph pass that already exists.
Signed-off-by: zengbohan1 <310902929+zengbohan1@users.noreply.github.com>
* fix(gateway): address review findings on checkpoint retention service
Resolves the review at a479cfe (willem-bd):
- Untested savers now fail fast: an explicit isinstance allowlist
(InMemorySaver / AsyncSqliteSaver / AsyncPostgresSaver) raises
NotImplementedError before any row is read or deleted, so a shallow or
third-party saver can never issue partial DELETEs.
- The chain walk ends (break) instead of raising KeyError when the head's
ancestor row is missing, matching the deletable loop's tolerance for
missing parents.
- enforce_thread_retention takes an optional per-thread lock and documents
the concurrency requirement: classification and deletion are two separate
passes, so callers must serialize per-thread mutation (runtime
_checkpoint_thread_lock) or guarantee quiescence.
- Dropped the dead mid-run guard: CheckpointTuple has no `next` field in
langgraph-checkpoint 4.1.1, and pending_writes is populated for committed
writes too (verified on the list path), so neither is a usable mid-run
signal; the caller-held thread lock is the actual protection.
- Removed the write-only _node_step/_Node.step and fixed the head-selection
docstring (newest by checkpoint id, not (step, checkpoint_id)).
- Documented the E1 leaf / history fast-path interaction in the contract doc
and module docstring: the wiring PR must sequence retention away from
history reads or adopt a policy that spares cache-carrying leaves.
- Added regression tests: unsupported saver, missing ancestor row, thread
lock parameter.
Validation: test_checkpoint_retention_service 18 passed / 8 postgres-gated
skipped; contract + lineage suites 18 passed / 6 skipped; ruff check and
format clean.
* fix(retention): count non-empty writes dicts on memory saver
- _checkpoint_ids_with_writes now requires a non-empty writes dict on
InMemorySaver: the empty phantom entry for checkpoints whose task wrote
nothing no longer counts as "owns writes rows", so the default E1 pruning
reaches the memory backend again (it was a silent no-op there).
- test_runtime_duration_leaf_pruned_by_default runs the shipping default
(strict_pending_write_guard=True) and proves E1 is reachable out of the
box on every backend; the stale override and its wrong SQLite premise
are dropped.
- document that _checkpoint_thread_lock is non-reentrant: a caller already
holding it must not pass it in, or retention self-deadlocks.
* test(checkpoint-retention): fix stray duplicated def token in test_duration_link_protected_after_next_run
The previous push left `async def def test_...` at line 244, which made the
module unimportable and failed collection of the whole suite (and ruff
format --check). Local copy was already correct; this commit re-pushes the
clean file. 18 passed / 8 postgres-skipped verified from a head worktree.
* fix(gateway): make retention correct on Postgres and fail closed on a bad cap
* validate max_delete_per_run before any store read: a negative cap used to
widen the batch (Python slicing) instead of being rejected;
* report identical before/after stats for an empty thread instead of returning
before stats_after is collected;
* protect each namespace's resume head and ancestor chain, so a persistent
subgraph's latest checkpoint is no longer treated as a sibling leaf;
* read Postgres columns through a row-factory-agnostic helper (the PG savers
open cursors with dict_row, where positional access raises KeyError: 0);
* classify the duration-only leaf without relying on metadata["writes"], which
the Postgres saver strips via get_serializable_checkpoint_metadata.
Verified locally on memory, SQLite and a real Postgres 16 instance (62 passed,
0 skipped): the E1 shape now fires on Postgres, which no backend test covered
before CI ran the Postgres lig.
Signed-off-by: zeng-bohan <zengbh1@gmail.com>
* test(gateway): pin the Postgres-shape duration classifier; report per-namespace heads
- Deterministic regression for _mark_duration_leaves_without_the_marker:
hand-put the Postgres round-trip shape (writes marker popped, source=
update + accumulated run_durations + channel_versions identical to the
parent) and assert the shipping default prunes it; a control that bumps
one channel version (the client update_state shape) with otherwise
identical metadata stays protected. Both legs run on memory and SQLite,
so the class cannot silently re-widen (a resumable head losing head
protection) or re-narrow (E1 never firing on Postgres) without a
locally-executing test failing.
- RetentionReport.protected_head_id -> protected_head_ids: heads are now
selected per namespace, so the report carries every namespace's head
(root key = what an unsaved aget_tuple resolves) instead of only the
global max - reshape it before the wiring PR starts consuming reports
for audit/aggregation.
---------
Signed-off-by: zengbohan1 <310902929+zengbohan1@users.noreply.github.com>
Signed-off-by: zeng-bohan <zengbh1@gmail.com>
Co-authored-by: zengbohan1 <310902929+zengbohan1@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(frontend): gate tool-step links through the href scheme allowlist
The chain-of-thought renderer turned web_fetch args and web_search /
image_search result URLs straight into <a href>. Markdown links already
pass isSafeHref, but these tool-step links bypassed it, so a
prompt-injected tool call could put file:, ms-msdt:, vscode: or other
OS protocol-handler links into the chat. React 19 only rewrites
javascript: hrefs.
All three sites now reuse the markdown allowlist and render an unsafe
URL as plain text (the image thumbnail stays, unlinked). Tests render
MessageGroup for each tool with unsafe schemes plus a web-URL control.
* docs(changelog): note tool-step link scheme gating (#5526)
* fix(frontend): mark omitted tool-step links and guard web_fetch url type
Review follow-up. Tool steps dropped an unsafe URL to bare text, while
markdown and artifact links show a dotted "Unsafe link omitted" span, so
the two surfaces applying the same rule degraded differently. That span
was already duplicated between markdown-link.tsx and artifact-link.tsx;
it is now one UnsafeLink component used by all three renderers. It
passes extra props through so the image tile still works as a Radix
tooltip trigger.
web_fetch also read args.url with a cast only. A non-string url (models
occasionally emit one mid-stream) reached JSX as an object and threw,
taking down the message list. It is now typeof-guarded.
* fix(frontend): default missing tool-call args before rendering steps
Review follow-up. The web_fetch typeof guard dropped the optional
chaining of the cast it replaced, so a tool call without an args object
threw again. Other branches were already exposed the same way: seven
tool kinds (web_fetch, web_search, image_search, read_file, write_file,
str_replace, browser_*) threw on a missing or null args while building
their labels. convertToSteps now defaults args to {} once, so every
ToolCall branch receives an object.
0023_run_change_seq was chained ahead of the already-shipped
0023_user_preferences revision, so databases stamped at that revision or
later treat it as an applied ancestor and never execute it: the
run_change_clock table and runs.change_seq column are permanently missing
and the first thread deletion fails with 'no such table:
run_change_clock' (#5516). 0025_repair_run_change_seq re-applies the same
guarded DDL on upgrade and no-ops on healthy shapes. RunChangeClockRow and
UserPreferenceRow are also registered in the ORM model registry.
Fixes#5516
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(gateway): preserve owner isolation when thread metadata is missing
Follow-up to the #5448 review P1 (post-merge finding): owner_check=True
also authorizes threads whose meta row is missing (legacy compatibility)
or NULL-owner (shared/pre-auth data). _run_scope_user_id returned None
for every trusted internal caller, which dropped the only remaining
per-user filter on those threads and let an internal caller acting for
owner A read owner B's persisted runs.
_run_scope_user_id now takes the thread_id and consults the thread meta
store: when an existing meta row establishes ownership, the authorized
thread's runs are still read unfiltered (merged #5448 semantics,
including owner-header-less internal callers); when the meta row is
missing or NULL-owner, the filter falls back to the acting owner's raw
stamp (the exact value start_run writes) — or the synthetic "default"
identity without an owner header — so cross-user runs stay hidden.
Isolation coverage uses the real MemoryThreadMetaStore with no metadata
row (and a NULL-owner row) plus another user's persisted run: /runs and
/runs/page must be empty and /runs/{run_id} must 404 for internal
callers, while an established-ownership thread keeps the unfiltered
read.
* fix(gateway): gate run-scoped sub-resource reads for internal callers
Review follow-up on #5484: the P1 owner-isolation class remained
reachable through run-scoped sibling reads that apply no per-user filter
at all — /runs/{run_id}/messages, /events, /join, /stream and
/workspace-changes query by (thread_id, run_id) directly, so on
missing/NULL-owner threads an internal caller acting for owner A could
still read owner B's run content by id (verified 200 at the previous
head).
- Extract _thread_ownership_established (shared meta-row check) and add
_require_run_visible_to_scope: for internal callers on threads without
established ownership, the run's own user_id stamp must match the
acting owner's raw value (or the legacy "default" stamp) or the read
404s. Established-ownership threads and every non-internal caller keep
their existing thread-scoped semantics.
- Wire the gate into join, stream, messages, events and
workspace-changes; reword the now-stale messages comment to track the
new scoping semantics.
Regression tests: sub-resource reads 404 for a mismatched internal
owner while the matching owner reads them normally, and the owner-less
fallback branch (synthetic "default" filter on missing-meta threads) is
pinned. Red confirmed against the pre-gate head.
* fix(gateway): gate cancel and artifact archive for internal callers
Review follow-up on #5484 round 2: POST /cancel resolved runs unscoped
(require_existing=True only closes the missing-meta case — NULL-owner
meta rows still pass), so an internal caller acting for a different
owner could interrupt another owner's active run on a shared thread
while /join and /stream were already gated. The archive manifest and
download pair likewise leaked the other owner's delivered-file count
and a 200-vs-409 delivery oracle on NULL-owner threads (missing-meta
threads were already denied by require_existing=True).
All three routes now call _require_run_visible_to_scope; its docstring
records the extended coverage. NULL-owner-thread regression tests pin:
a mismatched internal owner gets 404 from cancel, manifest and archive
download, while the acting owner reaches the real conflict path (409 on
a terminal run) and reads the manifest (file_count 2).
* fix(gateway): tolerate state-less request stand-ins in the scope helpers
The new owner-isolation gate and _run_scope_user_id read request.state
directly, which crashed the FakeRequest-based unit suites for the run
events, workspace-changes and scope endpoints (backend-unit-tests shards
1/2/4 on #5484). Read the state object defensively first: a request
without state is simply not an internal caller, so those paths keep
their pre-gate semantics.
* fix(gateway): scope the thread token-usage aggregate by owner
Review follow-up on #5484 round 4: GET /{thread_id}/token-usage called
aggregate_tokens_by_thread(thread_id) with no user filter at all, so on
missing/NULL-owner threads an internal caller acting for owner A read
owner B's spend, model names, run count and (with include_active=true)
live activity; the NULL-owner variant reached browser sessions too.
build_context_usage's latest-model lookup was unfiltered as well.
aggregate_tokens_by_thread gains an optional user_id (mirroring
list_by_thread: explicit None = unfiltered, AUTO resolves the contextvar)
in the memory store, the SQL repository and the store base;
build_context_usage/_resolve_thread_model_name thread the scope through
the latest-run lookup; the token-usage endpoint passes
_run_scope_user_id's value. Established-ownership threads aggregate
unfiltered as before; shared/missing-meta threads narrow to the acting
identity. Stale helper-test comment reworded after the #5482 merge
adaptation.
* test(gateway): pin the unfiltered aggregate on established-ownership threads
Review follow-up on #5484 round 5: the established-ownership branch of
the token-usage scoping (store receives user_id=None) was the only
unpinned half of the contract — the round-4 call-assertions never set
app.state.thread_store, so their None came from the user-less stand-in
path. test_token_usage_unfiltered_on_established_ownership_for_
internal_callers seeds an established meta row plus runs stamped by two
different identities and asserts the totals fold (166 = 111 + 55);
together with the isolation tests it now catches both failure modes
(always-stamp narrowing and always-None leak).
