Repository navigation
Conversation
721068c to
f4a00d0
Compare
cuericlee
left a comment
There was a problem hiding this comment.
需要修复的问题
P1 — 建议合入前修复
- tool_runtime.py _schema_to_dsh_value() 中 additionalProperties 语义错误
additional = schema.get("additionalProperties")
return {
"type": "object",
"properties": rendered,
"additionalProperties": bool(additional), # ← 问题在这
...
}
JSON Schema 中缺省 additionalProperties 时默认为 允许 额外属性;这里 bool(None) → False,生成的 DSH 工具会比原 ADK FunctionTool 更严格:凡是 schema 未显式声明 additionalProperties 的函数(例如带 **kwargs 的 Python 函数),DSH 端会拒绝额外入参。应改为仅在 "additionalProperties" in schema 时才透传。建议补一个针对此行为的单测。
- 配置了持久 dsh_home 时,进程退出清理未注册
def _ensure_dsh_home_root() -> str: # 只有这里注册了 atexit
...
atexit.register(_cleanup_runtime_state)
...
runtime.py 中:
dsh_home = effective.dsh_home or _generated_dsh_home(key) # 持久 home 不经过 _ensure_dsh_home_root
atexit 清理只在生成临时 DSH_HOME 时注册。配置了持久 dsh_home 后,_MANAGERS 里的 manager 永远不会被 close,DSH 子进程在进程退出时不关闭——这与 spec「Managers are closed at process exit」和 CON-5 的生命周期承诺矛盾。建议把 atexit 注册移入 _get_manager(或首次创建 manager 处),保证任何路径下退出都会清理。
P2 — 建议本轮处理或明确记录
3. 取消时 close() 与并发 turn 的竞态
except asyncio.CancelledError 分支直接调 manager.close()(不持有 manager 的 async lock)。而同一 manager 的另一个 in-flight turn 可能正持有锁在使用 self._harness,close() 会把正在被使用的 harness 关掉,导致并发 turn 损坏。spec 已承认"无法优雅取消",但"取消一个 turn 弄坏另一个共享 manager 的 turn"比文档描述更严重。建议:取消时若锁被占用则只标记,或把 close 也纳入锁内、且取消 turn 不直接 close 共享资源。
- 持久 dsh_home 的多进程冲突
DeepSeekHarnessToolServer.create() 把工具插件写到 Path(dsh_home)/"profiles/node_modules/veadk-tools/index.mjs",其中嵌入了 per-process 的随机 token。若多个进程配置同一个持久 dsh_home(PRD 明确推荐持久 home 用于跨进程会话保留),后启动的进程会覆盖插件文件,先启动进程的 DSH 子进程加载到的是新 token,桥接调用将 401 失败。建议把工具插件放到进程私有目录(如 tmpdir),而不是共享的 dsh_home。
- 文档 support matrix tracers 行未提 deepseek-harness
tracers 行只写 "codex does add a call_llm span... one per turn",但 deepseek-harness 同样实现了 per-turn call_llm span(有 test_runtime_emits_call_llm_span_for_tracing 佐证,示例的 observability 检查也依赖它)。文档应补充 deepseek-harness,否则与实现不一致。
- 夹带无关测试改动
tests/frontend/service/studio_release_notifier/test_notification_workflow.py 的 builtins.import fake-time 重构与本 PR 主题无关,属于 scope creep。虽然是 test-only、低风险,但建议拆出或说明动机(是否因本次改动破坏了原 time.sleep monkeypatch 方式)。
- FR-9(sub-agent 转交)无单测
新增测试文件中没有针对 transfer_to_agent 的测试,PRD 验收也仅写 "Source review"。run_transferred_agent / 抑制最终文本 / _charge_llm_call 跳过这些分支是本 runtime 较复杂的路径,建议至少补一个 fake 级的转交测试。
P3 — 可选优化
==0.1.5rc1 硬 pin:PRD 已自认是风险,发布前需复核;建议在 pyproject 注释中标注版本升级流程。
manager key 不含进程环境:SDK 子进程默认继承进程 env,若用户通过环境变量(而非 config)注入凭据,env 变化后 manager 复用会拿到旧凭据。属文档化行为,但可在 docs 提示"凭据变化需换 manager key / 重启"。
request_timeout_seconds 默认 None:DSH run 无超时兜底,示例里才设 180s。建议默认给一个非空值或文档明确。
示例访问私有属性:main.py 用 tracer._inmemory_exporter._exporter,依赖内部实现,稍脆弱。
import hashlib 写在 from_bundle 函数体内:风格小问题,提到模块顶部即可。
桥接请求无读超时:readuntil(b"\r\n\r\n") 无超时保护(loopback + token 场景下风险低,可忽略)。
其他一致性确认(未见问题)
compat.py 规则矩阵与 docs 支持矩阵一致:skills_mode 只对 codex/piagent 告警(DSH 桥接 skill);enable_skills_checklist 对 DSH 仍告警且措辞准确;codex_runtime_config / deepseek_harness_runtime_config 互斥告警对称。
get_runtime 缺 extra 时的 ImportError 提示正确且精确匹配 deepseek_harness 前缀。
provider 别名映射(大小写不敏感、空值归一、未知名透传)与 CON-4a 一致。
_dsh_session_id 用 SHA-256 而非原始 session id,符合 spec 的不可信输入处理。
测试数量与 PRD 验收记录吻合(15 passed + parity 1 个 = 16)。
f4a00d0 to
c8207a2
Compare
|
@cuericlee P1
P2
P3
验证
|
Summary
Adds
runtime="deepseek-harness"support to VeADK, backed by the DeepSeek Harness SDK/DSH runtime path. The implementation keeps the existing PiAgent and Codex runtime behavior intact while adding a new runtime adapter, event translation, tool execution bridge, skill loading, memory support, observability integration, docs, examples, PRD/specs, and regression tests.What Changed
Added DeepSeek Harness runtime support:
veadk.runtime.deepseek_harnessAdded install support:
deepseek-harnessoptional extra inpyproject.tomldeepseek-harness-sdkAdded runtime registration:
runtime="deepseek-harness"/ related aliases resolved through VeADK runtime compatibility pathsAdded examples and docs:
examples/deepseek_harness_runtimeUpdated shared tool bridge logging:
veadk.runtime.piagent.tools_bridgenow supports configurableruntime_labelpiagentlogsdeepseek_harness_*User Usage
Install with the DeepSeek Harness extra: