* fix(skills): offload update_skill's blocking IO and share the config write lock
Re-applied on top of main rather than merged: update_skill has since gained
admin gating, user-scoped storage and a PUBLIC vs CUSTOM/LEGACY split, so the
offload is applied per path.
- PUBLIC: the extensions_config.json read-modify-write (path resolve, snapshot,
merge, write, reload) moves to a worker thread via asyncio.to_thread. The
payload is built from a snapshot so the cached singleton is never mutated in
place while the write is still in flight.
- CUSTOM/LEGACY: set_skill_enabled_state is offloaded; the non-user-scoped
fallback takes the same shared-file RMW path as PUBLIC.
- Both load_skills calls (and storage construction) are offloaded.
The RMW lock now lives next to reload_extensions_config as
get_extensions_config_write_lock() and is acquired by both the skills router and
the MCP router, which performs the same read-modify-write on the same file.
Previously each router held its own module-local lock, so once both sides
offloaded, a PUT /api/mcp/config could run inside a skill toggle's read->write
window and the later write would silently drop the other's change.
The lock is keyed by the running event loop rather than being a module-level
singleton: asyncio primitives bind to the first loop that awaits them, which
makes a plain module-level lock unusable in a process that runs more than one
loop.
Adds a cross-router regression anchor asserting a skill toggle and an MCP update
never overlap inside the RMW (max in-flight 1); it observes 2 when the routers
use separate locks.
* fix(config): own the extensions_config RMW lock from the worker thread
An asyncio.Lock held around `await asyncio.to_thread(...)` protects only the
awaiting task. If that task is cancelled the context manager releases the lock
immediately while Python keeps running the worker thread, so a second skills or
MCP writer could acquire it and operate on extensions_config.json concurrently
with the first worker — reopening the lost-update window this was meant to close.
The per-event-loop keying had a second hole: writers on different loops got
different locks and so did not exclude each other at all.
Replace it with a process-wide threading.Lock acquired *inside* the worker that
performs the RMW (`_write_extensions_skill_state` and `_apply_mcp_config_update`),
so ownership belongs to the thread doing the writing and is held until the write
and reload actually finish, regardless of what happens to the caller. A
threading.Lock also has no event-loop affinity.
Adds a cancellation regression: the skills worker is paused inside the lock, its
route task is cancelled, and the MCP writer is started — the MCP RMW must not
enter until the skills worker is released. Against the previous asyncio-lock
design this test fails with ['skills-enter', 'mcp-enter'].
The two existing serialization tests instrumented by replacing the worker
functions, which now bypasses the lock under test; they instead patch inside the
real workers (each module's reload_extensions_config, the last step under the
lock) so the production lock is exercised.
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(mcp): offload blocking filesystem IO in MCP config update
update_mcp_configuration resolved the extensions config path, probed its
existence, read the raw JSON, wrote the merged config, and reloaded it — all
blocking filesystem IO on the event loop (PUT /api/mcp/config). The whole
read-modify-write after the async admin check has no interleaved awaits, so it
moves into one _apply_mcp_config_update helper dispatched via asyncio.to_thread;
the masked response is built on the loop. The secret-preserving merge, error
codes, and the stdio command allowlist are unchanged.
Found via `make detect-blocking-io`. Same class as #3457 / #3529 / #3551.
Add tests/blocking_io/test_mcp_router.py anchor, verified red->green under the
strict Blockbuster gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(mcp): serialize concurrent config updates with a write lock
Address review on #3552: offloading the read-modify-write to a worker thread
dropped the implicit serialization the single-threaded event loop provided, so
two concurrent PUT /api/mcp/config calls could interleave and clobber each
other. Guard the offloaded RMW with a module-level asyncio.Lock to restore
within-process atomicity (cross-process writers remain a separate, pre-existing
concern).
Add a serialization regression test (red->green: without the lock the tracked
max concurrency exceeds 1).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: address mcp config blocking io review
---------
Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(channels): harden runtime credential management APIs
* fix(channels): address review feedback on credential hardening
Follow-up to the runtime credential-hardening pass, resolving five review
findings:
- WeChat auth persistence now writes through a 0o600 NamedTemporaryFile +
Path.replace instead of write_text-then-chmod, so the iLink bot_token is
never briefly readable at umask defaults (mirrors ChannelRuntimeConfigStore).
- The post-write chmod is split into its own try/except: a chmod failure on a
filesystem without POSIX perms now logs at debug instead of masquerading as
a "failed to persist" warning.
- Extracted the three near-identical _require_admin_user helpers (mcp,
channel_connections, channels) into a single require_admin_user(request, *,
detail) in app/gateway/deps.py; each router supplies its own detail string.
- Strengthened the runtime-config-store chmod coverage: a new test injects a
temp-file chmod failure and asserts it is logged at debug while the
destination is still owner-only (mutation-verified to fail if the chmod is
dropped), plus a loose-pre-existing-file case.
- Removed the unused _FakeRepo from the blocking-io test: its isinstance gate
routes through the repo-less 503 path, so neither stub was ever invoked.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(security): mask sensitive values in MCP config API responses
GET /api/mcp/config previously returned plaintext secrets including
env dict values (API keys), headers (auth tokens), and OAuth
client_secret/refresh_token. Any authenticated user could read all
MCP service credentials.
This commit masks sensitive fields in GET/PUT responses while
preserving the key structure so the frontend round-trip (GET masked
→ toggle enabled → PUT) correctly preserves existing secrets.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(security): address Copilot review on MCP config masking
- Load raw JSON (un-resolved $VAR placeholders) as merge source instead
of resolved config, preventing plaintext secrets from replacing
$VAR placeholders on disk (Comment 2)
- Preserve all top-level keys (e.g. mcpInterceptors) in PUT, not just
mcpServers/skills (Comment 1)
- Reject masked value '***' for new keys that don't exist in existing
config, returning 400 with actionable error (Comment 3)
- Allow empty string '' to explicitly clear OAuth secrets, while None
means 'preserve existing' for safe round-trip (Comment 4)
- Add 3 new tests for rejection, clearing, and edge cases (18 total)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: extract shared utils to break harness→app cross-layer imports
Move _validate_skill_frontmatter to src/skills/validation.py and
CONVERTIBLE_EXTENSIONS + convert_file_to_markdown to src/utils/file_conversion.py.
This eliminates the two reverse dependencies from client.py (harness layer)
into gateway/routers/ (app layer), preparing for the harness/app package split.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: split backend/src into harness (deerflow.*) and app (app.*)
Physically split the monolithic backend/src/ package into two layers:
- **Harness** (`packages/harness/deerflow/`): publishable agent framework
package with import prefix `deerflow.*`. Contains agents, sandbox, tools,
models, MCP, skills, config, and all core infrastructure.
- **App** (`app/`): unpublished application code with import prefix `app.*`.
Contains gateway (FastAPI REST API) and channels (IM integrations).
Key changes:
- Move 13 harness modules to packages/harness/deerflow/ via git mv
- Move gateway + channels to app/ via git mv
- Rename all imports: src.* → deerflow.* (harness) / app.* (app layer)
- Set up uv workspace with deerflow-harness as workspace member
- Update langgraph.json, config.example.yaml, all scripts, Docker files
- Add build-system (hatchling) to harness pyproject.toml
- Add PYTHONPATH=. to gateway startup commands for app.* resolution
- Update ruff.toml with known-first-party for import sorting
- Update all documentation to reflect new directory structure
Boundary rule enforced: harness code never imports from app.
All 429 tests pass. Lint clean.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore: add harness→app boundary check test and update docs
Add test_harness_boundary.py that scans all Python files in
packages/harness/deerflow/ and fails if any `from app.*` or
`import app.*` statement is found. This enforces the architectural
rule that the harness layer never depends on the app layer.
Update CLAUDE.md to document the harness/app split architecture,
import conventions, and the boundary enforcement test.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add config versioning with auto-upgrade on startup
When config.example.yaml schema changes, developers' local config.yaml
files can silently become outdated. This adds a config_version field and
auto-upgrade mechanism so breaking changes (like src.* → deerflow.*
renames) are applied automatically before services start.
- Add config_version: 1 to config.example.yaml
- Add startup version check warning in AppConfig.from_file()
- Add scripts/config-upgrade.sh with migration registry for value replacements
- Add `make config-upgrade` target
- Auto-run config-upgrade in serve.sh and start-daemon.sh before starting services
- Add config error hints in service failure messages
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix comments
* fix: update src.* import in test_sandbox_tools_security to deerflow.*
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: handle empty config and search parent dirs for config.example.yaml
Address Copilot review comments on PR #1131:
- Guard against yaml.safe_load() returning None for empty config files
- Search parent directories for config.example.yaml instead of only
looking next to config.yaml, fixing detection in common setups
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: correct skills root path depth and config_version type coercion
- loader.py: fix get_skills_root_path() to use 5 parent levels (was 3)
after harness split, file lives at packages/harness/deerflow/skills/
so parent×3 resolved to backend/packages/harness/ instead of backend/
- app_config.py: coerce config_version to int() before comparison in
_check_config_version() to prevent TypeError when YAML stores value
as string (e.g. config_version: "1")
- tests: add regression tests for both fixes
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: update test imports from src.* to deerflow.*/app.* after harness refactor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
