refactor(cli): SOLID + DRY — wizard god-file, yaml/envfile parsing unification, global state

[hrygo]

Background

internal/cli/ 模块包含 44 个 Go 文件（~6,748 行），横跨 checkers/（诊断检查注册表）、onboard/（安装向导）、slack/（Slack 客户端辅助）和 output/（终端格式化）。模块已在 cycle 37-42 完成 12 方面分析（issues #213, #230, #248）。本次为 重新分析（增量深度），聚焦 Phase 1 SOLID + DRY，重点发现 跨包重复代码——手写 YAML 解析和 .env 文件操作分散在 3 个子包中。

Scope: solid, dry — cycle 104 (重新分析, analysis_count=7)
Key files: onboard/wizard.go, checkers/config.go, checkers/runtime.go, checkers/dependencies.go, onboard/templates.go

---

Finding Summary

Category	Critical	High	Medium	Low
solid	0	1	1	0
dry	0	0	4	0
Total	0	1	5	0

---

Findings

SOLID

wizard-god-file

Severity: High | Confidence: High | ROI: High
Location: onboard/wizard.go (1378 行, 15+ 职责)

Problem: wizard.go 是一个 1378 行的 god-file，混合了 15+ 个不同职责：环境预检、运行模式检测、向导步骤编排、显示/banner/prompt 辅助函数、YAML 配置生成、.env 文件写入、STT 依赖管理、二进制安装、服务安装、agent 配置部署、文件 I/O 工具（sameContent、fileSHA256、copyBinary）、PATH 操作和配置验证。低级文件工具（SHA256、二进制拷贝、PATH 操作）与高级向导编排混在同一文件中。

Current Pattern:

// wizard.go:1270 — cryptographic file comparison
func sameContent(a, b string) bool { ... }

// wizard.go:1293 — atomic binary copy with temp file
func copyBinary(src, dst string) error { ... }

// wizard.go:1334 — shell PATH manipulation
func addToUserPath(dir string) error { ... }

// wizard.go:170-287 — Run() mixes orchestration + step definition + display
func (w *Wizard) Run() error { ... }

Proposed Fix: 拆分为 4-5 个文件：

wizard.go   — Run(), wizardContext, buildSteps() 编排（~300 行）
steps.go    — 所有 stepXxx() 函数（~300 行）
prompts.go  — 所有 prompt*() 和 display*() 辅助（~200 行）
binary.go   — copyBinary, sameContent, fileSHA256, PATH 操作（~150 行）
envfile.go  — loadEnvFile, readExistingEnv*, buildEnvContent（~150 行）

Acceptance Criteria:

• wizard.go 缩减至 ~300 行（编排 + 上下文定义）
• 文件工具提取到 binary.go 或 envfile.go
• make test 通过

---

global-configpath-dip-violation

Severity: Medium | Confidence: High | ROI: Medium
Location: checkers/config.go:18-23, 引用点在 config.go:32,109,148,215、security.go:87,169,217,227,327、stt.go:90

Problem: checkers 包使用包级全局 var configPath string + SetConfigPath() 模式。所有配置依赖的 checker 通过隐式全局状态获取配置路径，而非通过接口注入。这导致 configPath == "" 守卫重复 8+ 处，测试需要手动调用 SetConfigPath() 且有污染风险，无法并发运行不同配置路径的检查集。

Current Pattern:

// checkers/config.go:18-23
var configPath string
func SetConfigPath(path string) { configPath = path }

// 每个 config 依赖的 checker 中：
func (c configExistsChecker) Check(ctx context.Context) cli.Diagnostic {
    if configPath == "" {
        return cli.Diagnostic{ /* ... */ }
    }
    // ...
}

Proposed Fix: 通过 ConfigProvider 接口注入：

type ConfigProvider interface {
    ConfigPath() string
    EnvPath() string
}

type CheckerRegistry struct {
    checkers []Checker
    cfg      ConfigProvider
}

Acceptance Criteria:

• 提取 ConfigProvider 接口
• checker 通过接口获取配置路径，消除全局变量
• make test 通过

---

DRY

handrolled-yaml-parsing-3x

Severity: Medium | Confidence: High | ROI: High
Location: checkers/config.go:332-358, onboard/templates.go:227-269, onboard/wizard.go:559-590

Problem: 3 个独立函数手工逐行解析 YAML 文件，各用不同的字符串拆分逻辑。这种实现脆弱（注释、多行值、不同缩进会破坏），且修改 YAML 解析策略需同步 3 处。

Current Pattern:

// config.go:332-344 — extractPortAddr
lines := strings.Split(yaml, "\n")
prefix := key + ":"
for _, line := range lines {
    trimmed := strings.TrimSpace(line)
    if strings.HasPrefix(trimmed, prefix) { ... }
}

// templates.go:227-269 — extractBlock
// 手工缩进计数查找块边界

// wizard.go:559-590 — readExistingConfigValue
// 手工两级 YAML 嵌套状态机解析

Proposed Fix: 创建 yamlutil 共享 helper：

// yamlutil/get.go — 使用已有的 gopkg.in/yaml.v3
func GetValue(configPath, dottedKey string) (string, error)
func GetBlock(configPath, parentKey string) (string, error)

Acceptance Criteria:

• 创建 yamlutil 共享 helper（基于 yaml.v3）
• 3 处手工解析改为调用 helper
• make test 通过

---

envfile-parsing-7x

Severity: Medium | Confidence: High | ROI: Medium
Location: onboard/wizard.go:111,544,817,945、checkers/config.go:409-422、checkers/security.go:333-377、slack/client.go:65-92

Problem: 7 个 .env 文件读写实现在 3 个子包中分散。每个重新实现 KEY=VALUE 解析，引号/空白/注释处理各不相同。loadEnvFile 在 wizard.go 和 slack/client.go 间完全重复（已知 issue 212）。其余变体（readExistingEnvValue、readExistingEnvCredentials、hasEnvValue、fixEnvVars 内联解析）也是相同模式的不同封装。

Current Pattern:

// 7 处共享的模式：
data, _ := os.ReadFile(envPath)
for _, line := range strings.Split(string(data), "\n") {
    line = strings.TrimSpace(line)
    if line == "" || strings.HasPrefix(line, "#") { continue }
    idx := strings.Index(line, "=")
    // ... 提取 key/value ...
}

Proposed Fix: 创建共享 envfile 包：

// envfile/envfile.go
func Load(path string) map[string]string
func GetValue(path, key string) (string, bool)
func HasValue(path, key string) bool
func SetValues(path string, kv map[string]string) error
func UnsetKeys(path string, keys ...string) error
func LoadIntoEnv(path string) int

Acceptance Criteria:

• 创建 envfile 共享包
• 7 处实现改为调用共享包
• make test 通过

---

writability-check-boilerplate

Severity: Medium | Confidence: High | ROI: Medium
Location: checkers/runtime.go:169-221, checkers/dependencies.go:52-101

Problem: dataDirWritableChecker 和 sqlitePathChecker 实现完全相同的可写性检测逻辑：stat 目录 → 检查是否目录 → 创建临时文件 → 关闭删除 → 报告结果。仅目录路径和诊断元数据不同。

Current Pattern:

// runtime.go:169-221 和 dependencies.go:52-101 — 相同的模式
info, err := os.Stat(dir)
if err != nil {
    if os.IsNotExist(err) {
        return cli.Diagnostic{Status: cli.StatusWarn, FixFunc: func() error { return os.MkdirAll(dir, 0o755) }}
    }
    // ...
}
testPath := filepath.Join(dir, ".hotplex_write_test")
f, err := os.Create(testPath)
// ...

Proposed Fix: 提取共享 checkDirWritable helper：

func checkDirWritable(dir, name, category string) cli.Diagnostic {
    // stat → isDir → tempFile → cleanup → Diagnostic
}

Acceptance Criteria:

• 提取 checkDirWritable 到 checkers/helpers.go
• 两个 checker 共用
• make test 通过

---

Implementation Priority

Finding	Priority	Effort	Risk	Impact
handrolled-yaml-parsing-3x	P0	Small	Low	消除 3 处脆弱解析，统一为 yaml.v3
envfile-parsing-7x	P0	Medium	Low	消除 7 处跨包重复
wizard-god-file	P1	Large	Low	1378 行拆分为 5 个文件
global-configpath-dip-violation	P1	Medium	Medium	消除全局可变状态
writability-check-boilerplate	P2	Small	Low	消除 2 处样板代码

推荐起始点: P0 两个发现（yaml 统一 + envfile 统一）可在一个 PR 中完成，彻底消除跨包解析重复。

---

Out of Scope

• checkPythonPackages / checkOnnxPackage 重复 — Low 严重性，仅 2 实例
• displayExistingConfig Slack/Feishu 状态渲染重复 — Low 严重性，仅 2 平台
• loadEnvFile wizard.go vs slack/client.go 完全重复 — 已在 issue 212 中追踪
• configPath == "" 守卫 8+ 处 — global-configpath-dip-violation 的症状，根因修复后消除
• scanDir() agentconfig.go 样板 — 3 个一行方法，ROI 低

---

Verification

• make test 通过，无回归
• make lint 不产生新警告

[hrygo]

arch-audit: 已验证此 issue 仍然有效（代码仍匹配描述的模式）。确认。cycle 128.

[aaronwong1989]

arch-analysis (cycle 11, solid/dry/coupling): 补充发现 — checkers 包内 SRP + STT/TTS DRY。

config-checkers-srp-five-types-in-one-file

Severity: Medium | Confidence: High | ROI: Medium
Location: checkers/config.go (463 lines)

问题: 单文件包含 5 个 checker 类型（configExists, configSyntax, configRequired, configValues, configEnvVars）+ 5 个 fix/helper 函数。configEnvVarsChecker 和 fixEnvVars() 处理的是环境变量而非配置文件，属于不同关注点。与 security.go 中的 writeEnvVar/unsetEnvVar 存在功能重叠。

建议: 将 configEnvVarsChecker 和 fixEnvVars() 移至 checkers/env.go，将 env 文件工具提取到共享位置。

---

stt-tts-structural-duplication

Severity: Medium | Confidence: High | ROI: Medium
Location: checkers/stt.go (161 lines), checkers/tts.go (177 lines)

问题: STT 和 TTS 检查器共享几乎相同的结构：主 checker → requirements() 加载配置 → exec.LookPath 检查 → Python 包验证 → 平台特定安装提示。ttsInstallHint() 直接回退调用 stt.go 的 installHint()，创建了跨文件依赖。

代码片段:

// stt.go: 标准模式
type sttEnvironmentChecker struct{}
func (c *sttEnvironmentChecker) Check(ctx context.Context) cli.Diagnostic {
    cfg := loadConfig() // 加载配置
    deps := sttRequirements(cfg) // 计算依赖
    // LookPath + exec.CommandContext 检查
    return cli.Diagnostic{...}
}

// tts.go: 相同结构
type ttsEnvironmentChecker struct{}
func (c *ttsEnvironmentChecker) Check(ctx context.Context) cli.Diagnostic {
    cfg := loadConfig() // 相同
    deps := ttsRequirements(cfg) // 相同模式
    // LookPath + exec.CommandContext 检查 — 相同
    return cli.Diagnostic{...}
}

建议: 提取共享的 dependencyChecker 基础结构，STT/TTS 只提供差异化的依赖列表和安装提示。

---

security-go-env-utilities-misplaced

Severity: Low-Medium | Confidence: High | ROI: Low
Location: checkers/security.go:326-377

问题: envFilePath()、writeEnvVar()、unsetEnvVar() 是通用 .env 文件操作工具，定义在 security checker 文件中但被 config.go 的 fixEnvVars() 使用。这些函数与安全检查无关。

建议: 提取到 checkers/envutil.go 或共享工具包。

[hrygo]

arch-analysis (cycle 102, dry): 补充发现。

loadEnvFile 第 4 处重复

Location: `cmd/hotplex/gateway_run.go:655-683`
问题: `gateway_run.go` 中的 `loadEnvFile()` 是第 4 处与 issue 中 envfile-parsing-7x 追踪的实例完全相同的实现。之前追踪 3 处（cron/client.go, slack/client.go, wizard.go），现在确认 `cmd/hotplex/` 入口点还有一处在 `loadConfig` 中使用。

影响: 修正后的实例计数为 8 处（4 个 loadEnvFile + 3 个 checker 内的 .env 解析 + 1 个 fixEnvVars），修复范围应包含 `cmd/hotplex/gateway_run.go`。