Open
Conversation
…ixes FireRedTeam#11) - Add should_inline_media_as_base64() strategy (default False for local deployment) - load_media: send path relative to media_dir; fallback to absolute on ValueError - compress_payload_to_base64: no-op when local mode - BaseNode._load_item: path-only branch with media_dir resolution and traversal check - BaseNode._pack_item: return path-only when input was path-only (orig_md5 None) Resolves httpx.ReadError / ClientDisconnect when loading many media files. Made-with: Cursor
…remote - should_inline_media_as_base64(server_cfg) now reads local_mcp_server.connect_host - 127.0.0.1, localhost, ::1 -> path-only (local) - Any other host -> base64 (remote MCP) Made-with: Cursor
…update PR description Made-with: Cursor
…media docs Made-with: Cursor
…es auto-searched media not readable) Made-with: Cursor
…fallback Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Collaborator
|
辛苦了!感谢你对这个 PR 的耐心迭代,我们这边会尽快给出详细的review |
0e3c2b6 to
8de7bca
Compare
jcl-2026
reviewed
Mar 11, 2026
| # Dict: recursively process nested data (without saving) | ||
| loaded_input[k] = self.load_inputs_from_client(node_state, payload_input, user_info, save=False) | ||
| # Dict: if it looks like a file-path payload, load it like an item; otherwise recurse. | ||
| if self._looks_like_file_path(payload_input.get("path")): |
Collaborator
There was a problem hiding this comment.
此处原代码无需改动:
- _looks_like_file_path 不是一个必要改动,path目前暂没有兼容http这种格式。
| packed_output[k] = [self._pack_item(node_state, item) for item in payload_output] | ||
| elif isinstance(payload_output, dict): | ||
| packed_output[k] = self.pack_outputs_to_client(node_state, payload_output) | ||
| # Dict: if it looks like a file-path payload, pack it like an item; otherwise recurse. |
Collaborator
There was a problem hiding this comment.
与Comment on line R210 相同
| meta_collector: NodeManager = context.node_manager | ||
| input_data = defaultdict(list) | ||
|
|
||
| server_cfg = getattr(context, "cfg", None) or getattr(context, "server_cfg", None) |
Collaborator
There was a problem hiding this comment.
- server_cfg 变量表意不对,context对应<class 'open_storyline.agent.ClientContext'>,client_cfg更合理。
- getattr(context, "server_cfg", None) 可以删去
Made-with: Cursor
Contributor
Author
|
三处不合理的地方已经修改 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
这次改动老实说比我一开始想象的复杂得多。因为它同时牵涉到:
search_media)下载素材再进入同一条剪辑链路。每一步都和「素材是 path 还是 base64」、「路径相对哪个目录」有关,所以非常容易在某个场景上思考不周、踩到边界。
目前这个版本我已经在本地实际跑过多个流程:无论是上传本地视频剪辑,还是网络搜索素材剪辑,都可以完整跑通且不报错,并且在同机/path-only 模式下,本地文件处理的速度快了非常多。
不过我也必须承认,系统场景很多,依然有可能在我没覆盖到的情形下出现 bug,还请维护者帮忙多看一眼,有任何不合理的地方我都会再跟进修正。
0. 为什么新开 PR(v2)
之前的 PR 在多轮 review + 实测中不断补边界情况,提交历史变得分散,阅读/审核成本较高。为降低维护者的理解成本,我将最终确定需要的改动整理到一个干净分支并新开 PR(v2),聚焦解决同一个问题(issue #11),同时把实测踩到的关键边界一并补齐。
1. 这个 PR 主要解决什么问题(issue #11)
当 Agent 与 MCP server 在同一台机器(本地 / 单机 / 同机部署)时,现有实现会把媒体文件 gzip+base64 内联进 MCP HTTP 请求体(包括
load_media及依赖节点的 payload)。素材多/文件大时请求体会非常大,容易引发连接中断(例如ReadError/ClientDisconnect),从而导致load_media失败。本 PR 的目标是:同机默认走 path-only(不内联 base64)以避免超大 payload;跨机仍可走 base64;并且保证 path-only 模式在真实工作流(Web UI 上传、search_media 下载、BGM/资源路径、嵌套 dict path)下可用。
2. v2 相比 v1 多解决了什么
除了“避免同机巨型 base64 payload”这个核心点,v2 额外补齐了这些在实测中会导致“找不到素材/渲染失败”的边界:
inline_media = auto|always|never,并用同一策略函数驱动客户端/服务端决策。load_media优先生成project.media_dir相对路径,否则回退绝对路径,避免服务端按错误基准解析相对路径。search_media的产出路径合并进load_media.inputs,并兼容list[{"path":...}]与list[str]。media_dir下的资源路径回退绝对路径,避免下游错误解析。path统一 pack/load:不仅处理list[dict],也处理dict中的{"path": ...},减少“结构不同就漏处理”。3. 具体怎么解决(逻辑/实现)
3.1 单一策略来源:
should_inline_media_as_base64(server_cfg)inline_media(src/open_storyline/config.py)与config.toml示例项。inline_media=always强制 base64;never强制 path-only;auto由connect_host判断(127.0.0.1/localhost/::1/0.0.0.0视为本地)。3.2 客户端侧:只在需要时才内联 base64
compress_payload_to_base64(payload, server_cfg)在 path-only 时是 no-op;只在策略要求 base64 时才会把path压缩/编码并写入base64/md5字段。3.3
load_media的 path-only 输入生成 + 合并search_media在
ToolInterceptor.inject_media_content_before()中:inputs时:优先project.media_dir相对路径,否则绝对路径兜底(兼容 Web UI session 目录)。search_media的下载产出路径到load_media.inputs(兼容{"path": ...}/"...path..."),并对路径去重。3.4 服务端侧统一合约(输入解析 + 输出打包)
在
BaseNode中:_load_item:project.media_dir下解析并防止逃逸(路径穿越保护)。_pack_item:media_dir相对路径;若目标不在media_dir(如 BGM/资源)则返回绝对路径,避免下游再按media_dir误解析。pack_outputs_to_client/load_inputs_from_client:{"path": ...}也执行同一套 pack/load(不再只覆盖list[dict]),覆盖更多节点输出形态。Test plan
load_media(path-only)可正常读到素材。search_media→load_media(path-only)包含下载素材,后续节点可使用。select_bgm+render_video(path-only)BGM 路径可正确解析,渲染可正常跑通。python -m compileall src。再次感谢维护者之前在 PR #49 里给的详细建议(尤其是对“本地/path-only 合约”和“单一策略函数”的讨论),也感谢这次愿意再帮我 review 这个 v2 版本。很可能还会有v3,v4版本。如果还有我没有考虑到的场景或更合理的设计方向,也请直接指出,我会继续跟进修正。也非常非常非常非常感谢你们的耐心,我从这次改进中学到很多的内容。麻烦你们了,不胜感激🙏!