Align the feedback router's dependency injection with the schedule
slice's ScheduleServiceDep shape: a FeedbackServiceDep alias in deps.py,
handlers declaring the service as a parameter instead of resolving it
inline. Swapping the provider is now a one-line change and parameter
order stays unconstrained. The two conditional lookups in thread_runs.py
deliberately keep the imperative call -- they only need the service when
a page actually has an AI message to decorate, and a declared dependency
would 503 the whole listing on a memory backend.
Remove the legacy stack the hexagonal slice replaced, now that nothing
assembles it: app/scheduler/service.py (the old orchestration),
app/gateway/routers/scheduled_tasks.py (the old dict-returning router,
no longer mounted), the deerflow/scheduler package (its cron/timezone
rules live in ScheduleSpec), the dict-returning repositories in
persistence/scheduled_task*/sql.py, and the deps.py providers and
app.state wiring that served them. The ORM rows and the
uq_scheduled_task_run_active partial unique index stay -- the table
definitions live with the shared engine/alembic infrastructure and the
schedule adapters are their only readers and writers.
The legacy test suites go with the code they pinned; every scenario has
a counterpart in the new suites. The one suite that guarded semantics
rather than the old implementation -- the real-database dispatch-race
TOCTOU tests -- is migrated to the new stack as
test_schedule_dispatch_race.py, driving ScheduleService over the real
SQL adapters with the same barrier, natural-timing, and index-semantics
cases.
Docs and comments that named the old classes as the current wiring
(backend/AGENTS.md, reload_boundary.py, channel/service comments) now
name the composition-root wiring instead.
Add domain/schedule/commands.py with one frozen dataclass per HTTP-driven
write use case -- CreateScheduledTask, UpdateScheduledTask, PauseTask,
ResumeTask, DeleteTask, TriggerTask -- and make the service methods their
handlers, keeping the naming chain aligned across all three spellings
(command / handler / <Command>Request). The clock stays an explicit now=
handler parameter: it is a rule input, not part of the client's intent.
UpdateScheduledTask expresses partial updates with an UNSET sentinel, so
absence is unambiguous; the wire keeps its historical None-means-omitted
convention and the request model's to_command owns the translation. The
former ContextChange moves from service.py into commands.py unchanged.
The request models are renamed to <Command>Request and own transformation
① (to_command): identity is injected server-side and pinned by a test to
never appear on the wire models. Clock- and callback-driven writes
(run_once, dispatch_task, handle_run_completion, reconcile_on_startup)
deliberately stay plain methods -- those drivers have no wire shape to
translate, their inputs are already domain vocabulary.
The context package now also exports the commands and the service, which
completes its public API and retires the stale 'service not landed' note.
Move model/errors.py up one level to domain/schedule/exceptions.py, a
sibling of the model, matching the AWS domain layout the spec mandates
(exceptions/ is its own member of the domain folder, not part of the
model) and the feedback reference implementation. Class names keep the
PEP 8 Error suffix. Pure move -- the nine classes are AST-identical to
the originals; imports across the domain, adapters, router, and tests
now take errors from deerflow.domain.schedule.exceptions.
Resolves the alembic head fork: main introduced 0010_run_cancel_request
on 0009 while this branch carried 0010_feedback_tags -> 0011_..._drop.
The unmerged branch revisions are renumbered and rechained after main's
published one (0010_run_cancel_request -> 0011_feedback_tags ->
0012_feedback_drop_message_id), and every test head pin moves to 0012.
Wiring moves out of deps.py::langgraph_runtime into
app/composition.py::build_domain_services, so the rule the assembly owns
-- a memory database backend yields no services and the routes answer
503 -- is an assertion in tests/test_composition.py instead of a comment
inside the lifespan.
- command-ify the write use cases (RateRun / RetractRunRating; queries
keep plain parameters, commands stay dumb data)
- split the domain errors into exceptions.py, a peer of model.py
(PEP 8 Error suffixes, AWS-style module name)
- unify the aggregate->row mapping as _apply(row, feedback) so one
explicit field list serves both the insert and the update path
- drop the unused feedback.message_id column (migration 0011): feedback
is bound to a run, nothing ever wrote or read the field
- pin remove_for_run's equality semantics for user_id=None in the
contract suite and fix the port docstring that contradicted both
implementations
* fix(runs): show run duration once per run and label it as elapsed work
turn_duration is the run's wall-clock lifetime (updated_at - created_at),
but both message endpoints stamped it onto every AI message of the run
and the frontend rendered each stamp as 'Thought for X seconds' — the
same number repeated per message, and tool-wait time presented as model
thinking latency (#4152).
- share one stamping helper between list_messages and list_run_messages:
only the run's final non-middleware AI message carries turn_duration
(list_run_messages already tried to do this but iterated reversed()
without stopping at the first match)
- completed-state trigger copy becomes 'Worked for X seconds' since the
number includes tool execution, not just thinking
Fixes#4152
* fix(threads): stamp turn_duration once per run in /history replay too
get_thread_history stamped turn_duration on every AI message of a run
instead of only the last one, so a run with several AI messages (e.g.
a tool call followed by a final answer) showed the same duration badge
more than once on reload.
Rebasing onto main picked up #4118, which replaced the /history
duration path with a checkpoint-metadata fast path plus an
event-store/run-manager fallback for legacy checkpoints - both of
which stamped every AI message per run_id, reintroducing this same
bug on both paths. This commit folds the once-per-run stamping
(stamp_turn_duration_on_last_ai) into both paths instead of only the
legacy one, and narrows the fast-path/fallback boundary to a per-run
completeness check (turn_run_ids - checkpoint_run_durations) instead
of a per-message one, so a run whose duration is already in checkpoint
metadata never re-triggers the event-store correlation fallback just
because its non-final AI messages correctly have no turn_duration.
* fix(frontend): stop caching a client-measured turn_duration per message
A run can produce several standalone content-only AI messages (e.g. a
subagent handoff), each briefly becoming the newest message and each
mounting its own Reasoning timer via the shared turnStartTime. Wiring
onTurnDurationChange let that per-message timer cache and display a
"Worked for X seconds" badge the moment a later message superseded it
- with a different, premature number, since the timer only measured
that message's own streaming window, not the run's total elapsed
time. That reintroduced the #4152 duplicate-badge bug through a path
the backend fix (once-per-run stamping) doesn't cover.
cachedDuration now only ever mirrors a backend-confirmed
turn_duration (already handled by the other effect here), so a
superseded message can no longer display a stale or wrong duration.
* fix(frontend): use 'Worked for' framing even without a persisted duration
The completed-state copy diverged depending on whether turn_duration
had landed yet: "Thought for a few seconds" (no duration) vs. "Worked
for X seconds" (duration present). Both describe the same completed
run, just with or without a persisted number, so the framing
shouldn't disagree on what happened.
* docs(runs): note why the middleware skip is inert on /history, add test
stamp_turn_duration_on_last_ai's middleware-caller skip only has an
effect on the event-store message shape (metadata.caller); checkpoint
messages replayed on /history never carry that field. Document why
that's not a gap - middleware writes go to thread metadata, not the
messages channel, so no middleware message ever reaches a
checkpoint's messages list.
Also lock the middleware-skip contract on list_thread_messages with
its own regression test, mirroring the existing one for
list_run_messages - the thread feed only gained this skip through the
same shared helper, and previously stamped every AI message,
middleware included.
* chore(frontend): retain current run duration UI
The frontend behavior was superseded by #4348; keep the latest main implementation while retaining this PR's backend fix.
* test(frontend): retain current reasoning coverage
Align the old PR test with the frontend implementation already merged in #4348.
---------
Co-authored-by: fancyboi999 <fancyboi999@users.noreply.github.com>
A browser client served from a different origin than the Gateway never
learns the id of the run it just created, so a brand-new thread keeps its
placeholder route for the whole session and every action gated on an
established thread — edit and rerun, regenerate, branch — stays hidden
until the page is reloaded.
Run-creating routes return the run's id in `Content-Location`, and the
LangGraph SDK resolves run metadata from that header alone. It is not
CORS-safelisted, so a cross-origin response hides it from JS unless the
server lists it in `Access-Control-Expose-Headers`. `useStream`'s
`onCreated` therefore never fires and the app cannot rewrite its route.
Expose it. `GATEWAY_CORS_ORIGINS` is a supported deployment mode, so the
CORS middleware has to carry everything that mode needs to read. Same-origin
nginx deployments are unaffected because CORS never applies to them.
* feat(checkpoint): make delta snapshot_frequency configurable
* fix(config): carry legacy checkpoint_delta_snapshot_frequency with warning
Addresses review on #4516: the rename from the flat
database.checkpoint_delta_snapshot_frequency key to nested
database.checkpoint_delta.snapshot_frequency silently dropped the old
value (pydantic extra="ignore"). Add a before-validator that maps the
legacy key onto the nested one with a deprecation warning (nested key
wins when both are set), plus a CHANGELOG breaking-change note covering
the rename and the 1000 -> 10 default change.
* fix(checkpoint): validate frozen snapshot frequency
* feat(lark): sidecar credential broker for sandbox lark-cli (Pattern B)
Removes the plaintext Lark credential mounts (appSecret + OAuth tokens)
from the sandbox container. A long-running broker sidecar owns lark-cli
and the per-user config/data dirs and serves the command surface over
Pod loopback; the sandbox gets only a forwarding shim on PATH, so the
raw credential files never exist in the sandbox filesystem.
- lark_broker.py: stdlib-only loopback broker (argv passthrough with
shell=False, server-injected credential env, bounded I/O) + shim
script constant + install-shim mode.
- docker/lark-cli-broker: init(install-shim) + serve image.
- provisioner: LARK_CLI_BROKER_IMAGE + provision_lark_cli_broker →
shim init container + lark-cli-broker sidecar (config/data mounted
sidecar-only); credentials dropped from the sandbox container;
/api/capabilities reports lark_cli_broker_image. Broker supersedes
the Pattern A init-container binary when both are configured.
- gateway: lark_cli_env_overlay(broker=True) omits config/data env;
sandbox_lark_broker_active() TTL-cached mode resolver; broker added
to sandbox_runtime_mode / readiness and the settings UI.
Opt-in and off by default (empty LARK_CLI_BROKER_IMAGE ⇒ no change).
Closes#4338
* fix(lark): address Pattern B broker review findings (#4501)
Follow-up to the sidecar credential broker addressing the PR #4501 review:
- shim: split the on-PATH lark-cli into a /bin/sh launcher + Python shim body
so broker mode fails loudly (exit 127, actionable message) instead of ENOEXEC
when the sandbox image ships no python3; interpreter pinnable via
DEERFLOW_LARK_BROKER_PYTHON. Launcher bakes in the shim's absolute path since
$0 is the bare command name when run off PATH.
- broker: drop the dead cwd payload field (broker can't see the sandbox FS) and
document the command-surface-only / no-file-IO limitation.
- broker: return a structured 500 JSON on unexpected exec errors so the shim
gets a meaningful message, not an opaque transport failure; set a handler
socket timeout to bound slow/stuck connections.
- broker: add an opt-in DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS denylist that
refuses secret-dumping subcommands before spawning the binary, forwarded from
the provisioner sidecar.
- gateway: tighten the per-bash-call broker probe timeout (1.5s) and cache
negatives longer (300s) so non-broker remote-provisioner users don't pay a
latency hit; guard the mode cache with a lock; drop the dead
_probe_provisioner_lark_cli_init_image wrapper.
- docs: remove the broken design-doc link from the broker README.
Adds tests for launcher python resolution, cwd omission, denylist enforcement,
500-on-error, hot-path probe timeout + negative caching, and provisioner
denylist-env wiring.
Editing the only turn of a thread reran the original prompt: the model
answered the question the edit was replacing while the UI showed the
edited text, and the edit vanished on reload.
The replay-base lookup decided whether a checkpoint predates the target
user message by message id alone. DynamicContextMiddleware re-keys the
first user turn to `{id}__user` mid-run, so every checkpoint written
before it holds the same prompt under an id the lookup cannot match. The
scan walked past those and anchored inside the run that produced the
turn — a checkpoint that still contains the original prompt and owns the
injection node's pending writes, which the replay then re-added after the
edited message.
Require the replay base to be a settled checkpoint (no pending tasks) in
both the lineage walk and the chronological fallback. That rule is
middleware agnostic: the first turn now anchors on the thread's empty
initial checkpoint and later turns on the previous run's tail, which also
drops the existing reliance on LangGraph discarding a stale `__start__`
write.
Edit replay additionally passes `head_checkpoint` so it resolves its base
lineage-first like regenerate does, and a replayed user message is
restored to its pre-swap id: replaying `{id}__user` into a state that has
no reminder yet makes the middleware treat the turn as already injected
and silently drops its date and memory block.
Frontend: a prepared replay masks the turn it supersedes, so the
optimistic-message baseline is taken from the post-mask human count. The
pre-mask count can never be exceeded when the replay puts exactly one
human message back, and on the first turn the runtime re-keys the
replacement message so identity comparison cannot stand in for the count.
Fixes#4531
`spec_column.py` and `spec_wire.py` were the same 60 lines twice, kept in
sync by a parity suite. Both are gone; what they did is now split along the
line that actually separates the two boundaries.
Why two files existed
---------------------
They were one module until the slice split it, and the reason given for the
split holds: a primary adapter must not import a secondary one, and the two
shapes are equal only by coincidence. But that argument only requires the two
*shapes* to be independent -- it does not require the parsing *rule* to be
written twice, and writing it twice is what needed `test_schedule_spec_parity`
to assert the two agreed, down to identical error text.
Splitting it properly
---------------------
`ScheduleSpec.from_primitives(schedule_type, *, cron, run_at, timezone)` takes
four strings, not a `Mapping[str, Any]` -- the mapping was the thing that kept
this out of the domain, and four strings carry no transport or storage format
with them. It owns the whole rule: unknown type, missing or non-string field,
unparseable `run_at`, and (via __post_init__, unchanged) 5-field cron and
resolvable timezone. Values are checked rather than trusted, since both
callers read data a client can influence.
Each adapter keeps only what is genuinely its own -- which two keys its format
uses -- as private methods on the class that owns the boundary, matching how
`SqlFeedbackRepository` and AWS's own ports-and-adapters sample put the
conversion inside the adapter rather than beside it:
SqlScheduledTaskRepository._spec_from_row / _spec_to_column
models.ScheduledTaskCreateRequest.to_schedule / models._spec_to_wire
The emit direction stays duplicated, deliberately: it is three lines per side
with no rule in it, and the two are *allowed* to diverge -- one is an HTTP
contract, the other a storage format. Asserting they stay byte-identical was
a constraint neither side asked for, so that suite is not replaced.
The router stops building value objects
---------------------------------------
`create` passes `body.to_schedule()`. `update` passes
`body.to_schedule(current.schedule)`, replacing eight lines that re-emitted
the current spec to the wire shape purely to read defaults back out of it;
omitted parts now come off the value object directly.
Tests
-----
`test_schedule_spec_parity.py` is deleted (162 lines). Its structural and
value cases moved to `TestFromPrimitives` in the domain suite -- stated once
now instead of parametrized over two implementations. Emit coverage was
already elsewhere: wire in `test_schedule_response_models`, column via the
repository round-trip in `test_schedule_fakes`.
One gap found while removing it: the `once` half of the update fallback had
no coverage on either side (both existing cases use cron), and it is exactly
the branch this commit rewrites -- `spec_to_wire(current)` round-trip to
`current.run_at.isoformat()`. Added
`test_a_timezone_change_on_a_once_task_keeps_the_same_instant`, and confirmed
by mutation that it is the only case that catches that branch breaking.
Behaviour is unchanged: same wire shapes, same normalizations (whitespace in
cron, trailing Z re-emitted as +00:00), same error messages, so the 422
details clients see do not move.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Switches production onto the hexagonal path. The legacy modules still
compile and still have tests, but nothing assembles them any more; deleting
them is the next commit, kept separate so it stays reviewable.
Composition root
----------------
`app/composition.py::build_domain_services()` is now the only place an
adapter is instantiated. It was extracted from `deps.py::langgraph_runtime`
rather than added to it: wiring there was tangled with engine startup,
orphan recovery and shutdown, so the one rule that governs it -- no SQL
backend means no service and the routes answer 503 -- could not be tested
without booting the whole application, and was held up by a single comment.
It is a pure function of already-built infrastructure, so that rule is now
an assertion. Feedback moved with it; doing this while adding schedule's
five objects costs one change instead of two.
Primary adapter
---------------
The router is protocol translation only. What is gone is the giveaway: cron
normalisation, `next_run_at` arithmetic, the re-arm rule and hand-written
ownership checks all now live in the aggregate. Domain errors map to status
codes through one table, so a new error surfaces as a 500 to be classified
rather than being swallowed by whichever `except` was nearest.
`spec_mapping` split in two (AWS's own layout puts the wire model under the
entrypoint that owns it, and a primary adapter must not import a secondary
one): `adapters/schedule/spec_column.py` for the JSON column,
`routers/schedule/spec_wire.py` for the HTTP body. The two shapes are equal
only by coincidence, so `test_schedule_spec_parity.py` runs every case
against both and compares their outputs and messages directly. Function
names differ per side so an import from the wrong one is visible.
Explicit responses
------------------
Routes returned the ORM row's `to_dict()`, leaking `user_id`,
`assistant_id`, `overlap_policy` and the two lease columns. The response
models publish exactly the field set the frontend declares -- asserted in
both directions, since an extra field is a leak and a missing one breaks a
client.
One wire detail was nearly changed by accident: Pydantic v2 serializes a UTC
datetime as `...Z`, while the legacy `coerce_iso` path emitted `+00:00`.
`UtcTimestamp` pins `isoformat()` so adopting a model does not silently
alter the wire format for every client parsing these.
Tests
-----
73 new cases: router behaviour driven through a real `ScheduleService` over
in-memory fakes (a mocked service would let the error mapping pass without
a domain error ever being raised), response shape, and the composition root.
Router mappings verified by mutation -- a wrong status code or a dropped
timezone fallback turns 9, 2 and 1 cases red respectively.
Two lifespan tests carried a `SimpleNamespace` config that predates this
change; `langgraph_runtime` now reads `config.scheduler`, so they were given
one. Tolerating the gap with `getattr` was rejected: `AppConfig.scheduler`
always exists, so the fallback would be unreachable in production and exist
purely to excuse an incomplete test double.
Full suite is back to its 24 pre-existing failures.
Renaming a conversation and then editing one of its turns reverts the
title to whatever it was before that turn ran, so the user's own name for
the thread is silently replaced by an older automatically generated one.
Edit replay resumes from the checkpoint before the edited turn, and that
checkpoint predates the rename. Regenerate already guards against exactly
this rollback by replaying the current title as graph input; the edit
replay path was added later and did not carry the guard over.
Replay the title the same way, but only when the replay base already has
one. An untitled base belongs to a thread the title middleware has not
named yet — pinning the current title there would keep a name generated
from the prompt this edit just replaced, and stop the middleware from
naming the rewritten turn.
Group them by bounded context instead of by technology, one file per
port, and align the directory name with the AWS Prescriptive Guidance
layout (entrypoints / domain-with-ports / adapters).
app/infra/persistence/feedback.py
-> app/adapters/feedback/feedback_repository.py owned persistence
-> app/adapters/feedback/run_lookup.py anti-corruption layer
`persistence/` promised a technology-first classification that its own
contents contradicted: RunStoreRunLookup lived there while its docstring
said "no new SQL". Splitting per port makes that distinction structural.
SqlFeedbackRepository and _tz_aware move unchanged -- verified by
comparing their AST against the original rather than by eye. run_lookup.py
additionally gains a RunStore annotation behind TYPE_CHECKING (the module
is imported lazily by the composition root, so this keeps the runtime
import cost at zero), a docstring stating that this context owns no table
and writes no SQL against it, and a TODO recording the condition under
which the body is replaced: when the run context publishes a contract of
its own, the RunLookup port itself does not move.
Each module docstring opens with a fixed marker so the two kinds of
secondary adapter stay greppable:
grep -rl "anti-corruption layer" app/adapters/
Filenames deliberately carry no sql_ / acl_ prefix: a prefix encodes an
implementation property, so switching storage would force a rename even
though the port -- and therefore the import path -- has not changed. The
class name already carries it. A prefix earns its place once one port has
several production implementations, which is not yet the case here.
app/infra/ held nothing else and is removed.
Rebase the feedback migration onto main's new chain tip again: main added
0009_webhook_dedupe (also chained after 0008_thread_operation_kind), so
0009_feedback_tags becomes 0010_feedback_tags with
down_revision=0009_webhook_dedupe, keeping the chain linear.
Conflicts resolved:
- Five head-pin tests take main's version with the pin bumped to
0010_feedback_tags.
- test_thread_messages_page.py keeps this branch's feedback_service
wiring over main's feedback_repo wiring, but stubs both service
methods so the thread-grouped path main added coverage for stays
stubbed (latest_per_run_in_thread alongside latest_for_runs).
- test_thread_messages_feedback.py keeps both sides' imports; each is
used (Feedback for the fixture, EditReplayVisibility for the run
manager stub).
Verified: 79 tests across the conflicted files plus the migration and
bootstrap suites, and 54 feedback tests, all pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(memory): integrate FTS5 retrieval adapter
* deps: add jieba as default dependency for Chinese tokenization
Without jieba, FTS5 unicode61 tokenizer treats entire Chinese sentences
as single tokens, making single-character or sub-phrase searches
impossible (e.g. '吃' or '油泼面' returns 0 hits against
'用户喜欢吃油泼面'). jieba segments Chinese text into meaningful tokens
before indexing.
* fix(memory): avoid treating hyphens as FTS5 operators
* feat(memory): make Chinese tokenization optional
* fix(memory): warm every requested retrieval scope
* fix(memory): close retrieval resources on shutdown
* fix(memory): close backend when shutdown flush fails
* fix(memory): recreate corrupt retrieval index
* fix(memory): tolerate partial retrieval rebuilds
* fix(memory): warm retrieval index in background
* fix(memory): preserve shutdown flush budget
* fix(memory): stop retrying partial lazy rebuilds
* fix(memory): close retrieval through storage
* refactor(memory): simplify retrieval scope limit
* docs(memory): clarify retrieval shutdown lifecycle
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Rebase the feedback migration onto main's new chain tip: main added
0008_thread_operation_kind (also chained after 0007), so
0008_feedback_tags becomes 0009_feedback_tags with
down_revision=0008_thread_operation_kind, keeping the chain linear.
The five head-pin test conflicts resolve to 0009_feedback_tags on top
of main's versions (preserving the new operation_kind assertions).
Verified: 42 migration/bootstrap tests, 54 feedback tests, and the
full frontend suite (797) pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(frontend): pin recent chats
* fix(threads): address pin-chat review feedback
- Stop bumping updated_at on metadata-only PATCH (pin/unpin) via a new
update_metadata(touch=False) path so unpinning no longer jumps a chat
to the top of the updated_at-sorted recent list.
- Narrow patchThreadMetadata to a ThreadMetadataPatchResponse matching
the Gateway's actual response (no values/context).
- Namespace the pinned metadata key as deerflow_pinned for consistency
with deerflow_sidecar / deerflow_branch.
- Cover touch/touch=False behavior in repo + router tests; document the
e2e mock's updated_at preservation now mirrors production.
* style(frontend): format thread utils test
* fix(threads): make pinned ordering server-side
* test(frontend): keep infinite-scroll fixture order stable
* test(frontend): stabilize lark reconnect e2e
* docs: clarify thread pin metadata contract
RunCreateRequest declared stream_resumable as None-only, but langgraph_sdk
defaults it to False (not None) and its payload filter only drops None, so
every SDK request carried "stream_resumable": false and was rejected with 422.
False means "no resumable stream", which is what DeerFlow already serves.
This broke every IM channel run (runs.stream and runs.create both send the
field; only runs.wait omits it) and any external langgraph_sdk client. The web
frontend was unaffected because its compatibility wrapper does not send it.
An explicit true still returns 422. The new regression test drives a real SDK
client to capture its default payload and posts that to the HTTP boundary, so a
future non-None SDK default cannot regress this silently.
Fixes#4466
* fix(gateway): stop persisting Overwrite wrappers into empty reducer channels on branch
Thread branching (and POST /state on a never-written channel) wraps copied
reducer values in Overwrite. Upstream BinaryOperatorAggregate.update seeds
an empty (MISSING) channel with values[0] verbatim without unwrapping, so
Union-typed channels (sandbox/goal/todos/promoted) stored the wrapper
literally and the next consumer crashed with TypeError: 'Overwrite' object
is not subscriptable (#4380). Patch the channel to unwrap the first write
(mirroring DeltaChannel semantics), and stop copying thread-scoped channels
(sandbox, thread_data) into branches: the parent's sandbox_id would bind
the branch to the parent's workspace and release lifecycle.
* refactor(checkpoint): drop private _get_overwrite import for a local Overwrite check
Importing langgraph's underscored _get_overwrite at module top level meant an
upstream refactor that drops it - plausibly the same release that fixes the
bug - would fail this module's import and crash startup before the probe can
stand the patch down. Replace it with a local helper on the public Overwrite
type, and fix two test docstring nits.
* refactor(checkpoint): write patch flags via their constants to avoid drift
Both saver patches read their "already patched" idempotence flag through a
module constant (_PATCH_FLAG / _BINOP_PATCH_FLAG) but wrote it as a hard-coded
attribute literal, so renaming the constant would silently break the guard and
double-apply the patch. Write via the same constant (setattr), dropping the
now-unneeded attr-defined ignores.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* feat: add lark cli integration
* fix: polish lark integration actions
* feat: support lark incremental permissions
* fix: detect lark authorization completion
* fix: harden lark integration install
* feat: expand lark auth scopes and reuse host auth in sandbox
Default lark auth to least-privilege (recommend=false, base sign-in only)
and expose the full set of lark-cli --domain business domains as native
--domain grants instead of a 4-domain read-only mapping. Resolve the
skill pack from the latest larksuite/cli GitHub release at install time
with content-hash integrity, and surface version/runtime drift in status.
Share the per-user lark-cli config/data profile between the Gateway
Settings auth flow and agent conversations by mounting the integration
dirs into the AIO sandbox and injecting the matching env for lark-cli
commands, with an allowlisted extra_mounts path in the provisioner/K8s
backend and traversal guards on integration paths.
* style: fix lint issues from ruff and prettier
Sort imports in the provisioner PVC test and re-wrap two long i18n
description strings to satisfy backend ruff and frontend prettier CI.
* fix(lark): address managed integration review feedback
* fix(frontend): stabilize integrations settings e2e
* test(sandbox): isolate remote backend legacy visibility check
* test: fix backend unit failures after merge
* Harden Lark integration review fixes
* Format Lark integration E2E test
* fix(lark): harden sandbox credential exposure and status disclosure
Address willem_bd's security review on PR #3971:
- Mount the per-user lark-cli config dir (long-lived appSecret) read-only
into the AIO sandbox; only the refreshable-token data dir stays writable.
- Redact host filesystem paths (install_path, cli.path) from
GET /lark/status and the config/auth complete responses for non-admin
callers, fail-closed on any auth error.
- Document the npm postinstall trade-off (--ignore-scripts is not viable
because @larksuite/cli fetches its platform binary in postinstall).
- Document the sandbox credential trust boundary in AGENTS.md and README,
pointing at the sidecar-broker follow-up (#4338).
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Branch creation seeds the new thread's run-event feed from its checkpoint
so inherited history survives the first run (#4380). Every seeded row
carried one shared run id, but run_id is a *turn* identity to the feed's
consumers, not a provenance tag: regenerating the inherited answer
resolves that row's run id as the superseded source, and
GET /messages/page then drops every row carrying it. One shared id for
the whole seed therefore deleted the complete inherited history on a
branch's first regenerate, leaving only the regenerated turn.
Group seeded rows into one synthetic run per inherited turn
(branch-seed-{thread_id}-{n}), a new turn opening at each persisted human
message — the same boundary a real run has, including the allowlisted
hidden ask_clarification reply, which resumes as its own run. Supersession
is then confined to the turn actually regenerated.
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(summarization): own the run model for compaction; bound failure
With summarization.model_name: null the summary model resolved to
config.models[0] while the executing model is selected per run; when they
differ and models[0]'s provider is broken (expired key, quota, outage)
compaction silently failed every triggered turn and context grew unbounded
until the main provider 400s the run (#3103's shape), even though the run's
own model was healthy.
Model ownership is now sourced from the builders, not re-derived at runtime:
- The lead, subagent, and manual /compact builders each pass the resolved run
model into create_summarization_middleware(run_model_name=...). The middleware
no longer reads runtime.context / get_config(), which do not carry a custom
agent's or a subagent's resolved model, so a custom-agent lead run and a
distinct-model subagent now summarize with their own model, not models[0] /
the parent's. Runtime re-resolution and the per-name model cache are removed.
- model_name: null summarizes with the run's own model; an explicitly configured
summary model generates and falls back to the run model on failure. The
fallback is built lazily after the primary fails and its construction is
guarded, so a broken fallback cannot skip a healthy primary or escape the
automatic failure boundary.
Failure is bounded and side-effect-safe:
- An empty or whitespace-only response is treated as a generation failure, not a
valid summary, so compaction never removes all history for an empty replacement.
- compact_state/acompact_state take raise_on_failure independent of force: the
manual /compact path always surfaces a generation failure (even force=false)
and routes it to the existing ContextCompactionFailed path (HTTP 500 ->
frontend error toast) instead of an unconsumed response reason. The automatic
path leaves compaction state unchanged.
- before_summarization hooks fire only after a replacement summary exists.
SummarizationConfig.model_name, config.example.yaml, and docs/summarization.md
document the final lead/subagent/manual ownership rules.
Part of RFC #4346 (section A). Evaluating fraction/triggers against the run
model's profile (profile ownership) is a separate follow-up.
* fix(summarization): manual /compact model ownership + fail-open construct/parse
Manual /compact carried only agent_name, so it derived the run model from the
custom-agent model or config.models[0] and missed the request-selected model the
run path uses (request -> custom-agent -> default). Carry model_name through
ThreadCompactRequest and the frontend compact call, resolve with the same
precedence, and move the custom-agent config read off the event loop (asyncio
.to_thread) with user_id so the strict blocking-IO gate is not bypassed by the
broad except.
Make one summary attempt own its full lifecycle so the fail-open boundary covers
construction and response parsing, not just invocation: build each candidate model
lazily and guarded (a raising constructor falls through to the healthy run model
instead of breaking agent construction), build the model_name:null primary from the
run model rather than config.models[0], and run response text extraction inside the
invocation try so a failing .text accessor falls back instead of escaping compaction.
Adds factory-level constructor-failure, response-extraction-failure (sync/async), and
route-path model-ownership tests.
Renumber the feedback tags migration to 0008 on top of main's
0007_scheduled_run_active_index so the alembic chain keeps a single
head; bootstrap head pins follow.
Queue comments received while a run is active, then submit one deduplicated follow-up after it finishes. Failed drains are requeued and watcher tasks stop cleanly with the channel manager.
* fix(skills): offload blocking filesystem IO in get_custom_skill_history
The GET /api/skills/custom/{name}/history handler ran its storage probes and the
per-skill .history read directly on the asyncio event loop:
get_or_new_skill_storage(), custom_skill_exists(), get_skill_history_file().exists()
and read_history() are all blocking filesystem IO. make detect-blocking-io flagged
the existence probe (routers/skills.py:224) as DIRECT_ASYNC.
Move the whole read into a nested sync function run via asyncio.to_thread; a None
return signals 404 (distinct from an empty history list). Behavior is unchanged.
Per the blocking-io-guard SOP:
- Candidate: get_custom_skill_history (FILE_METADATA, DIRECT_ASYNC) -> FIX+ANCHOR.
- Re-scan: the finding no longer appears for this handler.
- Anchor: tests/blocking_io/test_skills_router.py drives the real handler against a
real on-disk skill + history; teeth verified red (pre-fix) -> green (post-fix)
under make test-blocking-io.
Scoped to this self-contained read handler. rollback_custom_skill and update_skill
also touch blocking IO but interleave it with awaits (security scan / cache refresh)
and do a read-modify-write, so offloading them needs the asyncio.Lock serialization
treatment (cf. #3552) and is left as a separate fix unit.
* test: trim dead skills history setup
* fix(skills): use the user-scoped storage accessor in the offloaded history read
The merge with main left the offloaded reader calling get_or_new_skill_storage,
which is not defined in this module (ruff F821), so lint failed and the handler
would raise NameError at runtime. Use _get_user_skill_storage(config) — the same
accessor every other handler in this router uses.
Also update the regression test for the current route signature: the handler is
now admin-only and takes a Request, so the test supplies request.state.user
(mirroring tests/blocking_io/test_channel_runtime_config_store.py) and seeds the
history through the same user-scoped accessor.
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Resolve the one import conflict in thread_runs.py: keep main's new
checkpoint_lineage imports (#4358 regenerate-in-branched-threads fix)
alongside this branch's get_feedback_service (feedback domain-service
refactor). All other files auto-merged. Verified: backend feedback/
regenerate/branch/lineage tests (112 passed) and the full frontend suite.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The composition root wires FeedbackService with its SQL adapter; routers
become protocol translation only (domain errors map to 400/404/409).
BREAKING CHANGE: removes five feedback endpoints with no frontend
consumers (POST, GET list, GET stats, DELETE by id under /threads, and
GET /runs/{id}/feedback). PUT/DELETE now follow the mainstream chat UX:
idempotent upsert plus retract, echoed through the message-list payload.
