test(cli): nyxid-cli 覆盖率 63.85%→65.49%(+43 测试)

[kaiweijw]

摘要 / Summary

提升 nyxid-cli 单元测试行覆盖率:63.85% → 65.49%(+1.64pp,新增 43 个行为测试)。Refs #852, #850.

覆盖 commands/{node_credential,update_attestation,ssh}、node/{oauth,daemon} 的纯逻辑与可 mock(wiremock)路径。剩余未覆盖主要是集成测试范畴(live WS/PTY/process runtime、daemon 服务管理命令、device/browser 轮询循环、完整 Sigstore trust-root/证书/Rekor 验证),单测无法诚实覆盖——这是 CLI 单测可达上限的真实信号。无 #[ignore]/skip/弱化断言。

base = auto/test-coverage;CI 阈值上调留待 #856。

🤖 Auto-loop / codex-refactor-loop

⟦AI:AUTO-LOOP⟧

[kaiweijw]

🤖 PR859 测试质量审核：通过

TL;DR

• 这是什么: 我按 tests 角色审核了 PR 859 的 CLI 覆盖率补充。
• 结论: 新增测试主要断言业务行为和协议细节，没有发现必须阻塞合并的测试质量问题。
• 需要 maintainer 做什么: 可继续走合并流程；我没有提出 must-fix。

---

详细说明

本轮重点看新增测试是否只是凑行数、是否引入 sleep/delay pacing、是否新增 skip/manual、是否弱化既有断言，以及是否覆盖实际新增的行为面。这个 PR 的 diff 只在 5 个 CLI 源文件里追加测试，没有新增 production 逻辑；因此我把判断集中在测试是否对真实行为给出约束。

检查结果是通过：node_credential 的测试用 wiremock 约束节点解析后的 API 路径和 JSON body；ssh 覆盖 URL 构造、header、shell escaping、principal 解析；update_attestation 覆盖 bundle/证书链/tlog/长度编码等解析分支；daemon 覆盖 profile label、metadata round-trip 和配置解析；node/oauth 覆盖 OAuth 参数过滤、PKCE、callback 解码、catalog fetch、refresh/code exchange 请求语义。没有新增 #[ignore]、manual/skip 标记，也没有新增测试侧 sleep/delay。PR 工作树上 cargo test -p nyxid-cli 通过：927 个 unit test + 1 个 integration test，0 ignored。

---

📎 完整 codex 原始输出(存档备查)

---
pr: 859
role: tests
verdict: approve
---

## Verdict
Approve: the PR only adds CLI tests, and the added coverage is behavior-oriented, deterministic, and passes the CLI test suite.

## Evidence
- `cli/src/commands/node_credential.rs:203` exercises node-name resolution plus the exact credential push endpoint/body via `wiremock`; `:251`, `:293`, and `:315` cover pending-list and cancel routing without adding sleeps or skips.
- `cli/src/commands/ssh.rs:665`, `:690`, `:699`, `:752`, and `:764` assert concrete URL, header, escaping, and principal-resolution outcomes rather than "does not throw" checks.
- `cli/src/commands/update_attestation.rs:673`, `:679`, `:689`, `:703`, `:720`, `:752`, `:805`, and `:816` cover parser success and failure behavior for JSON integer parsing, bundle decoding, certificate chain extraction, transparency entries, SCT length encoding, and Rekor key IDs.
- `cli/src/node/daemon.rs:983`, `:1102`, `:1110`, `:1123`, `:1146`, `:1174`, and `:1183` assert daemon label safety, systemd quoting, profile metadata round-trip/fallback behavior, explicit config precedence, and launchd target composition; HOME mutation is guarded by the existing env lock.
- `cli/src/node/oauth.rs:807`, `:849`, `:863`, `:893`, `:912`, `:919`, `:937`, `:964`, `:981`, `:1018`, and `:1047` validate OAuth parsing, reserved-param filtering, PKCE output, callback decoding, catalog fetch auth/status handling, and token-exchange request/response semantics with real request matching.
- Diff review found no added `#[ignore]`, manual/skip markers, assertion loosening, or test-side sleep/delay pacing. The only `tokio::time::sleep` hits are pre-existing production OAuth polling code at `cli/src/node/oauth.rs:230` and `:464`.
- There are no net-new production lines in the PR diff, so the coverage obligation is for the newly added tests themselves. `cargo test -p nyxid-cli` passes on the PR worktree: 927 unit tests and 1 integration test, 0 ignored.
⟦AI:AUTO-LOOP⟧
REVIEW_DONE:859:tests:approve

⟦AI:AUTO-LOOP⟧

[kaiweijw]

🤖 架构审查通过：PR 859 无架构阻塞

TL;DR

• 这是什么：我是 architecture/CLAUDE 合规角度的独立审查结果。
• 结论：approve，PR 859 只在 cli/src 增加测试覆盖，没有引入生产架构变化。
• 下一步：maintainer 可继续合并流程；架构角度没有阻塞项。

---

详细说明

本次三点 diff 只触及 cli/src/commands/node_credential.rs、cli/src/commands/ssh.rs、cli/src/commands/update_attestation.rs、cli/src/node/daemon.rs、cli/src/node/oauth.rs 五个文件，新增内容都落在 Rust 的 #[cfg(test)] mod tests 测试模块里；这个模块只在测试编译时启用，不改变生产 CLI 路径。对照 CLAUDE/AGENTS，我没有看到 backend 的 handlers/、services/、models/ 分层、MongoDB 模型、API 路由、schema/protocol、node owner 规则或 host SSOT 边界被修改。

我还按 architect checklist 做了新增行 grep：没有新增 Old/New 或 iteration refactor-history 源码注释，没有 *WriteActor / *ReadActor / *Store 这种单业务实体拆分，没有外部 repo 引用，没有把 host 事实搬进 .refactor-loop 或 host.env，也没有新增空转发接口、死 wrapper、compat shim 或并行生产路径。

---

📎 完整 codex 原始输出(存档备查)

---

pr: 859
role: architect
verdict: approve

Verdict

Approve - no architectural concerns; the PR is scoped to CLI test coverage and does not change production architecture or project-rule boundaries.

Evidence

• cli/src/commands/node_credential.rs:162, cli/src/commands/ssh.rs:649, cli/src/commands/update_attestation.rs:631, cli/src/node/daemon.rs:928, and cli/src/node/oauth.rs:657 add coverage inside existing #[cfg(test)] mod tests blocks only; no backend handlers/ / services/ / models/ changes, MongoDB model changes, API route changes, or protocol/schema changes are introduced. Reviewed against CLAUDE clauses including "Strict separation: handlers/ -> services/ -> models/", "Admin node endpoints (handlers/admin_nodes.rs) require admin role and have no ownership check", and "nyxid node daemon subcommands manage background service lifecycle".
• Scope honesty: the three-dot diff touches only cli/src/commands/node_credential.rs, cli/src/commands/ssh.rs, cli/src/commands/update_attestation.rs, cli/src/node/daemon.rs, and cli/src/node/oauth.rs, which is within the implement prompt's cli/** only scope; no backend, Cargo, CI, .refactor-loop/host.env, or host production SSOT files are changed.
• Checklist greps over added lines found no new Old/New or iteration refactor-history source comments, no *WriteActor / *ReadActor / *Store business-entity splits, no external repo references, no .refactor-loop host-fact migration, and no empty forwarding interface, dead wrapper, compat shim, or parallel production pathway.

⟦AI:AUTO-LOOP⟧
REVIEW_DONE:859:architect:approve

⟦AI:AUTO-LOOP⟧

[kaiweijw]

🤖 quality review: comment

TL;DR

• 这是什么: PR 859 的 readability / simplicity 复审。
• 结论: comment; 测试新增整体聚焦，但有 1 个无效 clippy::await_holding_lock allow。
• 需要 maintainer 做什么: 可在合并前删掉该 allow；不是 reject。

---

详细说明

cli/src/node/daemon.rs:1124 给 daemon_config_metadata_round_trips_profile_config_dir 加了 #[allow(clippy::await_holding_lock)]，但这个函数是同步 #[test]，没有 .await。HomeGuard 和 env lock 的用途能从命名与 SAFETY 注释看懂，这个 allow 反而暗示存在异步持锁问题，建议删除。

---

📎 完整 codex 原始输出(存档备查)

---
pr: 859
role: quality
verdict: comment
---

## Verdict
Comment: the test additions are focused and readable overall, but one new Clippy allow is misleading noise because the test is synchronous.

## Evidence
- `cli/src/node/daemon.rs:1124` adds `#[allow(clippy::await_holding_lock)]` to `daemon_config_metadata_round_trips_profile_config_dir`, but the function is a synchronous `#[test]` with no `.await`; the allow does not document a real lint suppression and makes the env-locking setup look more complex than it is.

## What would change your verdict (only if comment or reject)
Remove the unused `#[allow(clippy::await_holding_lock)]` from `daemon_config_metadata_round_trips_profile_config_dir`; the `HomeGuard` helper and env lock are otherwise clear and reachable.

⟦AI:AUTO-LOOP⟧
REVIEW_DONE:859:quality:comment

⟦AI:AUTO-LOOP⟧

[kaiweijw]

✅ 评审共识 — MERGE_WITH_COMMENTS

architect ✅ approve · tests ✅ approve · quality 💬 comment(advisory,非阻塞)。本地独立验证 cargo nextest run -p nyxid-cli = 928 passed, 0 skipped(整合分支 PR 不触发 ci.yml,故本地验证为准)。合并到 auto/test-coverage。

🤖 controller consensus card

⟦AI:AUTO-LOOP⟧