-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(serve): allow relocating session attachment storage via env var #10066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e1644ba
da506f7
97e64bc
789b808
3b522e4
3c4072a
539f0d7
6f42945
0f3e4a3
35298df
c0eedf8
027a2a5
6610d34
5657260
6589015
4106ef3
702c665
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -711,6 +711,21 @@ Scope and limits: | |
| - Binary uploads (`POST /file/upload`) always create at `0600` regardless of this setting. | ||
| - The daemon reads the variable at workspace-filesystem construction; restart the daemon after changing it. | ||
|
|
||
| ### Session attachment storage | ||
|
|
||
| Session attachments (files and images uploaded by Web Shell through `POST /session/:id/attachments`) are stored by default under the workspace's runtime temp dir: `<runtimeBaseDir>/tmp/<projectHash>/attachments`, keyed per session as `session-<sessionId>`. Operators who want attachments to persist outside the runtime temp dir (e.g. on a dedicated volume) can override the root: | ||
|
|
||
| | Env var | Values | Default | What it does | | ||
| | ------------------------------------- | ------ | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `QWEN_SERVE_SESSION_ATTACHMENTS_ROOT` | path | unset | Stores session attachments under this directory instead of the default runtime temp dir. Accepts an absolute path, a path relative to the daemon's cwd, or `~` / `~/…` expanded against the home directory. | | ||
|
|
||
| Scope and limits: | ||
|
|
||
| - **One-way migration.** When the env is set, new attachments are written only under the configured root. Reads and removes that miss the configured root fall back to the default runtime temp dir, so attachments uploaded **before** the switch remain readable, and removable while the default fallback dir stays writable — a remove whose legacy copy cannot be unlinked (e.g. a read-only fallback volume) surfaces the error rather than reporting success. The reverse direction — removing the env after attachments were written to the configured root — makes those attachments unreachable; keep the variable stable for a given workspace. | ||
| - **Per-session layout.** Files live under `<root>/<projectHash>/attachments/session-<sessionId>/` in both locations, where `<projectHash>` is the same workspace hash used by the default runtime temp dir; the fallback lookup uses the same session layout in the default dir. Two workspaces pointing at the same configured root stay isolated from each other. | ||
| - **Delete cleanup.** When a session is deleted, its attachment directory is removed from both the configured root and the default fallback dir. Archiving a session keeps its attachments so they survive unarchive. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] This bullet promises more than the code delivers: the orphan-reap deletion path deletes sessions without ever deleting their attachments. Verified by execution with a real bridge: upload → orphan reap completes ( Concrete trigger: an ACP-HTTP Fix: in 中文说明该条目的承诺超出了代码实际行为:孤儿回收删除路径删除会话时从不删除附件。 实测(真实 bridge):上传 → 孤儿回收完成( 修复:在 — qwen3.8-max via Qwen Code /review (v0.22.0) |
||
| - The daemon reads the variable at startup; restart the daemon after changing it. The directory must be writable by the daemon process. | ||
|
|
||
| ## Multi-session & multi-workspace deployment | ||
|
|
||
| Pass `--workspace` more than once to register several non-overlapping workspaces in one `qwen serve` process. The first path is primary. Each registered workspace owns an isolated runtime boundary, while the daemon-wide listener, authentication policy, and total-session limit are shared. Production attempts to preheat the primary ACP child for compatibility and retries on first use after failure; trusted secondaries start their own child on demand, and untrusted secondaries do not start ACP. Requests may select a registered workspace by canonical `cwd`; requests that omit `cwd` use the primary workspace. Use one daemon per user or security principal; workspace trust is an execution gate, not an ACL. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] Reviewer Test Plan step 3 instructs verifying the REVERSE of the one-way migration this bullet documents: "Then remove the env var, restart, and confirm the same attachment is still readable (fallback path)". With the env unset,
sessionAttachmentsRoots()returns only{ root: defaultRoot }— no fallback is constructed (packages/cli/src/serve/session-attachments-root.ts) — so an attachment written to the configured root is unreachable by design and the step always fails. This bullet itself states the reverse direction makes configured-root attachments unreachable, and the PR's Risk & Scope section says the same. A reviewer following step 3 as written uploads withQWEN_SERVE_SESSION_ATTACHMENTS_ROOTset (bytes land under the configured root), unsets the var, restarts, reads the attachment — unreachable — and files a false fallback-regression against correct, test-pinned code, costing a review round. Fix: rewrite step 3 to exercise the shipped direction — upload an attachment with the env unset, then setQWEN_SERVE_SESSION_ATTACHMENTS_ROOT, restart, and confirm new uploads land under the configured root while the pre-switch attachment stays readable/removable via the fallback; note that unsetting the var afterwards makes configured-root attachments unreachable.中文说明
Reviewer Test Plan 第 3 步要求验证的恰是本条目所记载的单向迁移的反方向:"取消该环境变量、重启,并确认同一附件仍可读(回退路径)"。环境变量未设置时,
sessionAttachmentsRoots()只返回{ root: defaultRoot }——不会构造任何回退目录(packages/cli/src/serve/session-attachments-root.ts)——因此写入配置根目录的附件按设计就不可达,该步骤必然失败。本条目自身已写明反向操作会使配置根目录中的附件不可达,PR 的 Risk & Scope 部分也有同样说明。按第 3 步原文操作的评审者会在设置QWEN_SERVE_SESSION_ATTACHMENTS_ROOT时上传(字节落入配置根目录),然后取消变量、重启、读取附件——不可达——并对正确且有测试钉住的代码提交一份虚假的"回退回归"报告,浪费一轮评审。修复:把第 3 步改写为实际支持的方向——先在未设置环境变量时上传附件,然后设置QWEN_SERVE_SESSION_ATTACHMENTS_ROOT并重启,确认新上传落入配置根目录、同时切换前上传的附件仍可经回退目录读取/删除;并注明此后再取消变量会使配置根目录中的附件不可达。— qwen3.8-max via Qwen Code /review (v0.22.2)