refactor(worker): move protocol types to claudecode, implement WorkerCommander, harden OCS permissions#483
refactor(worker): move protocol types to claudecode, implement WorkerCommander, harden OCS permissions#483hrygo wants to merge 6 commits into
Conversation
- Rename .agent/ directory to .agents/ and copy all rules, settings, and progress logs - Update .claude symbolic link to point to .agents instead of .agent - Swap ignore rules in .gitignore to track .agents and ignore transient files - Update references to .agent/ in AGENTS.md and spec files in docs/specs/
…code package Move claudeUserMessage, claudeUserMsg, claudeTextContent types and SendUserMessage logic from base/conn.go to claudecode/input.go as unexported streamUserMessage types and writeStreamInput function. This resolves a SOLID/DIP violation where the shared base package contained Claude Code-specific protocol details. base.Conn now exposes Stdin() and SetLastInput() for protocol-specific adapters to use. - Remove 3 protocol types + SendUserMessage from base/conn.go (-35 lines) - Add Stdin() and SetLastInput() methods to base.Conn - Create claudecode/input.go with writeStreamInput package-level function - Migrate SendUserMessage tests to claudecode/input_test.go - Update claudecode/worker.go Input() to use writeStreamInput Refs #375
…ind/permission logic - Remove stale <claude-mem-context> block appended to AGENTS.md - claudecode: document Compact args intentionally ignored (unlike OCS) - claudecode: Rewind omits empty target_id instead of sending empty string - opencodeserver: initialize rules as non-nil slice (JSON [] not null) - opencodeserver: add slog.Warn when plan mode combines with allowed_tools - opencodeserver: pass AllowedTools through applyPermissions to setPermissionMode
hotplex-ai
left a comment
There was a problem hiding this comment.
PR #483 三维度审查报告
裁决:Request Changes
审查范围:57 files, +379/-199 — 协议类型迁移、WorkerCommander 实现、OCS 权限加固
FAIL 项(必须修复)
1. .agents/settings.local.json 提交了开发者个人路径和 PII [安全 + 代码质量 + 集成]
该文件包含 /Users/huangzhonghui/ 绝对路径和 MCP 工具权限配置。.gitignore 已有 .agent/settings.local.json(旧路径)但未添加 .agents/settings.local.json(新路径)。这是开发者本地配置,不应入库。
修复:将 .agents/settings.local.json 加入 .gitignore,从本次提交中移除该文件。
2. .agents/skills/ 多个 Skill 文件中 claude 被错误替换为 Codex [集成]
批量替换将 claude 相关引用错误替换为 Codex(OpenAI 产品名),与代码库不一致:
| 文件 | 错误 | 正确 |
|---|---|---|
hotplex-arch-analyzer/SKILL.md |
.Codex/arch-analysis/progress.json |
.claude/arch-analysis/progress.json |
hotplex-arch-analyzer/SKILL.md |
worker/Codex/ |
worker/claudecode/ |
hotplex-diagnostics/SKILL.md |
Codex --session-id |
claude --session-id |
hotplex-diagnostics/SKILL.md |
~/.Codex/projects/*/ |
~/.claude/projects/*/ |
hotplex-diagnostics/SKILL.md |
internal/worker/Codex/worker.go |
internal/worker/claudecode/worker.go |
hotplex-pr-quality/SKILL.md |
Co-Authored-By: Codex Opus 4.7 |
Co-Authored-By: Claude Opus 4.7 |
hotplex-release/SKILL.md |
Codex (commit scope) |
claude-code |
hotplex-setup/SKILL.md |
Codex/opencode 不在 PATH |
claude/opencode 不在 PATH |
代码库中 worker 目录名是 claudecode,CLI 是 claude,状态目录是 ~/.claude/。这些替换会导致 Skill 指向不存在的路径。
3. hotplex-release/SKILL.md 符号链接说明变为自引用 [集成]
AGENTS.md 是 AGENTS.md 的符号链接 ← 无意义
应恢复为:AGENTS.md 是 CLAUDE.md 的符号链接,只需编辑 AGENTS.md
4. allowedTools 循环在 setPermissionMode 中重复 4 次 [DRY]
for _, tool := range allowedTools {
rules = append(rules, map[string]any{"permission": "tool", "action": "allow", "pattern": tool})
}在 4 个 switch case 中完全相同。应提取为 switch 后统一追加。
WARN 项(建议改进)
5. Stdin() 存在 TOCTOU 窗口 [非功能性 · conn.go:667-671]
Stdin() 获取锁返回 *os.File 后释放锁,调用方 writeStreamInput 再次获取锁写入。两次锁操作之间 CloseInput() 可能使 fd 失效。当前通过 IsDeadProcessError 优雅降级,但值得后续改进为原子操作。
6. allowed_tools 工具名未做输入校验 [安全 · commands.go:~178-196]
工具名直接注入 OCS 权限 pattern,如包含特殊字符可能影响匹配行为。建议添加正则校验(^[a-zA-Z0-9_.-]+$)。
7. 测试不再验证自动 LastInput 捕获 [集成 · input_test.go]
旧测试验证 SendUserMessage 自动捕获 lastInput,新测试需要显式调用 SetLastInput。worker.go 中调用链正确,但缺少集成测试保障。未来改动可能遗漏 SetLastInput 调用。
8. AGENTS.md 删除了"配置热更新"备注 [集成]
该行描述的行为仍然准确且重要(参见 docs/guides/enterprise/multi-tenant.md),不应在格式清理中移除。
9. 贡献者指南仍引用已删除的 SendUserMessage [集成 · adding-worker.md:236-239]
SendUserMessage 已从 base.Conn 删除,但贡献者指南仍引导使用。需更新为 Stdin() + writeStreamInput 模式。
PASS 项
- 协议类型从 base 迁移到 claudecode — 正确的封装改进,单一职责原则遵循良好
WorkerCommander接口实现 —Compact/Clear/Rewind逻辑正确,Clear合理返回ErrNotImplementedStdin()/SetLastInput()并发安全 — 共享状态访问均正确加锁writeStreamInput错误处理 —fmt.Errorf("%w")包装、IsDeadProcessError分类正确- OCS
applyPermissions正确传递AllowedTools,plan模式有防御性 Warn - 新增
input_test.go104 行覆盖核心场景(JSON 格式、closed stdin、集成) - 无新增外部依赖,无 goroutine 泄漏,mutex 未嵌入未传指针
.agent/→.agents/重命名范围大但核心为纯重命名,风险可控
| 维度 | 代码质量 | 非功能性 | 集成防腐 | 裁决 |
|---|---|---|---|---|
| 结果 | PASS (2 WARN) | FAIL (1 FAIL + 2 WARN) | FAIL (3 FAIL + 3 WARN) | Request Changes |
…ments, DRY allowedTools - Remove .agents/settings.local.json from git tracking (contained developer paths/PII) - Add .agents/settings.local.json to .gitignore (new path after .agent→.agents rename) - Fix incorrect Codex replacements in 5 skill files (claude→Codex batch error) - Fix self-referential symlink description in hotplex-release/SKILL.md - DRY refactor: extract repeated allowedTools loop in setPermissionMode
Add missing modules (brain, cron, eventstore, service, updater, docs, webchat, sqlutil, assets), fix worker submodules (claudecode/base/proc), supplement messaging submodules (stt/tts/toolfmt) and CLI submodules (cron/slack), and align all descriptions with AGENTS.md.
Summary
base包迁移到claudecode包,消除 base 对特定 Worker 协议的耦合base.Conn新增Stdin()、SetLastInput()方法,支持 Worker 适配器直接访问 stdin 并记录 crash recovery 输入WorkerCommander接口:Compact(/compact)、Clear(not supported)、Rewind(rewind_files control_request)setPermissionMode支持allowed_tools白名单(B3-2 绕行),为每种模式生成 per-tool allow rulesCloses #375, Closes #433
Test plan
make check全部通过(fmt + lint + vet + build + tests)input_test.go:验证writeStreamInputJSON 格式、LastInput 捕获、closed stdin 错误处理、Stdin/SetLastInput 集成base/conn_io_test.go中SendUserMessage相关测试(已由新测试覆盖)/compact和 rewind 控制请求在 session 中正常工作