# PR #384 (provider-scoped binding group) 评审报告与场景对照

**日期**: 2026-07-26
**类型**: 评审 (review)
**评审对象**: [openxlings/xlings#384](https://github.com/openxlings/xlings/pull/384) —
`feat(xvm): add provider-scoped binding group registration`，分支 `fix/xvm-binding-groups`，
提交态 `origin/main...HEAD` = 10,234 insertions / 583 deletions / 13 files
**关联**: `2026-07-26-xvm-provider-binding-group-design.md`(PR 自带设计)、
`2026-07-26-xvm-provider-binding-group-plan.md`(PR 自带计划)、
`2026-07-26-xvm-group-transaction-design.md`(本评审导出的目标架构)、
`2026-07-26-xvm-group-transaction-plan.md`(实施计划与验收标准)

> **评审范围声明**：本文评审的是 **PR 的已提交态**。评审时 worktree 中另有 2,913 行未提交改动
> 及未跟踪的 `src/core/xvm/switching.cppm`(3,060 行)。那部分不在 PR 内，但它是 §3.1 结论的
> 唯一潜在缓解项 —— 若该文件随 PR 一并提交，§3.1 的结论需要重新评估。

---

## 0. TL;DR

| 维度 | 评级 | 说明 |
|---|---|---|
| 问题诊断 | **A** | 根因判断成立，且有代码与生态审计双重证据 |
| 数据模型设计 | **B+** | 方向正确、与工业实践同构；但 root 承载 manifest 是次优归一化 |
| 失败语义(fail-closed) | **B-** | 原则正确，但**强制先落地、恢复手段后落地**，顺序有风险 |
| 事务性 | **C+** | 内存态是真事务；磁盘与文件系统不是，PR 措辞过强 |
| 工程实践(文档/测试/边界声明) | **A-** | 生态审计、备选方案、Draft 边界声明都是高水准 |
| 可合并性 | **当前不可合并** | 存在 2 个静默功能回归(§3.1、§3.2)，必须先修；且 base 应改为 `release/0.4.70`(§6) |

一句话：**架构判断是对的，落地质量高于 xlings 现有平均水平，但这版提交里有两个会让用户"装完就坏"
的回归，且强制校验早于修复工具落地。**

---

## 1. 问题诊断：成立

PR 描述列出的 5 条根因，逐条对照 `origin/main` 实现，全部属实：

| # | 主张 | 代码证据(origin/main) |
|---|---|---|
| 1 | 缺少发布归属 | `src/core/xvm/db.cppm` 的 `VData` 只有 `path/alias/envs/includedir/libdir`，无 provider 维度 |
| 2 | 绑定边不完整 | `remove_version()` 只 `vers.erase()`，**完全不碰反向 `bindings` 边** —— 悬空边是必然而非偶然 |
| 3 | 删除权限错误 | 同函数在 bare version 下遍历所有 `ns:version` 匹配即删；`script.cppm` / `subos.cppm` 旧路径直接 `versions_mut().erase(name)` |
| 4 | 物化不在事务内 | `commands.cppm:198-215` 先 `remove_headers`/`install_headers` 再更新 workspace，中间失败留下撕裂状态 |
| 5 | 多入口不一致 | `commands.cppm:221-237` `collect_bindings` 的 visited key 是 **target 而非 `(target,version)`**，且 header/lib 只处理入口 target |

PR 自带设计文档 §3 那段生态审计（官方索引 37 个含 binding 的 recipe，逐条列出 e2fsprogs root
后注册、syslinux 自绑定、musl flavor root 版本错位、openssl 重复注册同一 exact 目标）是这份
工作最有说服力的部分 —— **先量化生态再定模型**，这是工业级做法。

---

## 2. 设计本身：与工业实践的对标

### 2.1 判断正确的部分

1. **把"发布"作为原子单位** —— 与 rpm/dpkg 的 source package → binary packages（共享
   `epoch:version-release`）、Nix derivation 的多 output（`out`/`dev`/`lib`）、conda 的
   build string 完全同构。选型正确，不是新发明。
2. **用 `(provider, providerVersion, group)` 而非 target 名或版本串做身份** —— 生态审计已证明
   后两者不安全（同一 alias 由 GCC/LLVM/musl/mingw 共同提供）。
   `provider = canonicalName`、`providerVersion = node.version`
   （`installer.cppm:178-181`）取自解析后的 `PlanNode`，来源权威。
3. **用 root manifest 取代成对边** —— 把隐式图收敛成单一权威记录。
4. **错误分类携带 JSON Pointer** —— `RegistrationError.path` / `BindingIntegrityIssue.path`，
   可诊断性远超同类项目，值得推广到 xlings 其他子系统。
5. **两阶段（全量校验 → 一次应用）+ shadow copy**（`registration.cppm:689` 全量拷贝 →
   校验 → `swap`）是教科书做法。
6. **文档先于代码，含备选方案 A/B 与拒绝理由，PR body 明确声明 Draft 边界与"未主张"的能力。**

### 2.2 结构性异议：manifest 不应挂在 root 成员上

当前身份是 `(provider, providerVersion, group) → rootTarget@rootVersion`，而 manifest
（`bindingMembers` / `bindingHeaders`）存放在 root 这个**同时也是成员**的节点上。代价直接
体现在错误分类里 —— `BindingErrorKind` 的 15 个值中有 5 个
（`RootReferenceMismatch`、`RootMissingFromManifest`、`GroupIdentityMismatch`、
`MemberReferenceMismatch`、`StartMemberMissing`）**纯粹用于维护 root 自指与成员反向引用
这组冗余不变量**，外加一个专门的 commit（`c51ee19 allow complete binding root migration`）
和 `bindings.cppm:183-195` 的 `non_root_metadata_error_()` 守卫。

**错误分类表本身就是"归一化不足"的证据。** 目标形态见
`2026-07-26-xvm-group-transaction-design.md` §2。

---

## 3. 具体缺陷（按严重度）

### 3.1 🔴 `includedir`/`libdir` 不再写入 → `xlings use` 的头文件/库切换静默失效

- origin/main `installer.cppm:740` 有 `vdata.includedir = op.includedir;`
- PR 中**该行被删除**，`RegistrationNode`（`registration.cppm:16-26`）也没有对应字段
- 但 `commands.cppm:206-215`（本 PR **未改**）仍然读 `vdata->includedir` 决定是否切换
- 新表示 `root.bindingHeaders` 在**提交态里零消费者**

安装时的 `install_headers` 仍会执行（`installer.cppm:1385` 的 effect 回放），所以第一次安装
可用 —— **故障只在切版本时出现**，是最难被发现的一类回归。

### 3.2 🔴 `headers` 从不携带 group → 合法 recipe 硬失败

`installer.cppm:202-205` 构造 header 时永远不设 `.group`：

```cpp
plan.batch.headers.push_back({ .sourceDir = operation.includedir });
```

而 `registration.cppm:563-572` 对无 group 的 header 要求 `groups.size() == 1`，否则
`HeaderAmbiguous` 直接失败。由于 `registration.cppm:436-467` 会给**每个未绑定节点各建一个
单例 group**，任何「`headers` + ≥2 个无 binding 的 `xvm.add`」的 recipe（例如 program 与 lib
分开注册）都会从「能装」变成「装不上」。Task 9（pkgindex recipe 审计）尚未落地，等于把兼容性
风险外推给了索引。

### 3.3 🟠 删除回退不做 group 一致性 —— 可复现本 PR 要消灭的那个 bug

PR 设计文档 §7.1 承诺：*"switches an active group to a **complete surviving group**… otherwise
clears all removed members **coherently**"*。

实际实现 `removal.cppm:312-325`：

```cpp
for (auto versionIt = installedIt->second.rbegin(); ...)   // 逐 target、按 installed[] 插入序取最后一个
```

**逐 target 独立挑选，无 provider/group 校验。** 删除 gcc@16 后，`gcc` 可能回退到 GCC 提供的
15，而 `g++` 回退到 musl 提供的 15 —— 精确复刻 PR 要修的混合工具链状态。

137 个新增 TEST 中**没有一条覆盖回退一致性**
（`grep 'TEST(.*[Ff]allback'` 只命中 downloader / theme / proxy）。

Task 5 计划了这件事，但**强制校验（本 PR）先落地、一致性回退后落地**，窗口期风险实际存在。

### 3.4 🟠 fail-closed 没有配套修复通道

- `vdata_from_json` 把解析问题记成 `bindingIntegrityIssues`，`vdata_to_json` **又把它写回磁盘** ——
  派生数据被持久化了
- `bindings.cppm:512-515` 见到任何 issue 一律拒绝
- `xlings self doctor --fix` 存在（`src/core/xself/doctor.cppm`），但**完全不认识 binding 元数据**
  （Task 6 未落地）

净效果：一个字段坏了 → 该版本永久不可解析 → 保存时把"坏"标记固化 → **用户只能手改
`versions.json`**。

工业界对照：`rpm --rebuilddb` / `dpkg --configure -a` / `nix-store --verify --repair` /
`git fsck` —— **强制校验从不单独发布，必须与修复工具同批。** 这是本设计最需要调整的工程排序。

### 3.5 🟡 "atomically" 名不副实

`apply_registration_batch` 的内存态确实全有全无，但：

- 落盘是 `Config::save_versions()` + `Config::save_workspace()` **两次独立写**，无 journal
- `platform.cppm:262` `write_string_to_file` = 裸 `fopen("w") + fwrite + fclose`，
  **无 temp+rename、无 fsync** —— 单次写本身就不原子
- 文件系统副作用在**提交点之后**执行，失败只 `log::warn(...) continue`（`installer.cppm:1376-1381`）
- `src/core/xvm/` **全目录无任何锁**（`git grep -n lock` 为空）

后两点叠加的后果是：本 PR **把并发丢更新的严重度从「状态过时」升级成「group manifest 指向
已删成员 → fail-closed → 工具链整体不可用」**。

PR body 的 "apply it atomically" 应改为
"single-batch validation with all-or-nothing in-memory apply"。

### 3.6 🟡 `snapshot_xpkg_removal_context` 的静默丢弃

`installer.cppm:382-386`：

```cpp
if (!providerContext) providerContext = std::move(*selection);
```

第二个属于**同 provider 但不同 release** 的 seed 被静默丢弃；`ProviderMismatch`（:371-380）
只比 provider 名、不比 providerVersion。下游会以别的错误码失败（属 fail-closed），但诊断信息
误导。这个 ~150 行函数（legacy singleton 特判、preferred seed、provider 优先、全库兜底扫描）
也是整份改动里唯一明显"在猜"的地方，与"不猜测"的原则存在张力。

### 3.7 🟡 `OwnershipConflict` 改变了安装的失败语义

`registration.cppm:607-617`：同一 exact `(target, version)` 若已被别的 provider 拥有，安装
**硬失败**。语义正确（这本就是冲突），但 main 上是静默覆盖，且当前失败只表现为
`log::warn` + `return false` —— 用户在默认日志级别下只看到 "install failed"，看不到原因，也
没有出口。

### 3.8 🟡 复杂度与性能备注（非阻塞）

- `resolve_legacy_graph_` 每次调用都重建全库入边索引（`bindings.cppm:329-339`），而
  `apply_registration_batch` 对每个 legacy 节点调用它一次（:668）、又对每个候选 group 调用
  一次（:810）→ O(N·E)。当前生态（37 个 recipe）无压力，但属于隐藏的二次项。
- `registration.cppm:619-628` 为每个节点做一次全库扫描找同组旧成员，同样 O(N·|DB|)。

---

## 4. 测试评估

`tests/unit/test_main.cpp`：4,391 → **10,336 行**，新增 **137 个 TEST**。

**强**：malformed metadata、跨 provider 冲突、精确删除、非对称边、legacy graph 拒绝、批量注册
回滚 —— 覆盖密度高，且全是失败路径测试，正是不变量类改动最需要的。

**弱**：

- ❌ 无 group 一致性回退测试（→ §3.3 未被拦住）
- ❌ 无「安装 → use → 头文件确实切换」的端到端回归测试（→ §3.1 未被拦住）
- ❌ `HeaderAmbiguous` 只测了"它会报错"，没测"真实 recipe 形状不该触发它"（→ §3.2 未被拦住）
- ⚠️ 不变量数量已到了值得上 **property-based / 随机 DB 形状 fuzz** 的规模；现在全是手写用例
- ⚠️ 单文件 10k 行的 `test_main.cpp` 本身是维护性问题

**三个 🔴/🟠 缺陷全部落在测试盲区里，且盲区形状高度一致：都是"正向兼容性"缺测。**
这个模式值得记入团队实践 —— 引入强校验的改动，正向兼容测试比失败路径测试更容易被漏掉。

---

## 5. 使用体验对照（修复前 / 只落地本 PR / 全部完成）

### 5.1 场景一：切换工具链版本（核心场景）

#### 修复前（origin/main）

```console
$ xlings install gcc@15.1.0
$ xlings install gcc@16.1.0
$ xlings use gcc 15.1.0
✓ gcc -> 15.1.0                 # 成功，退出码 0，没有任何警告

$ gcc --version
gcc 15.1.0
$ g++ --version
g++ 16.1.0                       # ← 静默错乱
```

用户此时**完全不知道自己处于坏状态**。后果被推迟：

- `g++ main.cpp` 报看不懂的 ABI / 头文件不匹配错误
- 或 `g++: error: ... No such file or directory`（workspace 写进了只存在于残缺边上的版本，
  shim 解析不到实体）
- `xlings list` 显示 workspace 内容，**它会跟着一起撒谎**
- sysroot 只换了入口 target 的头文件，libstdc++ 头文件还是 16 的

**最糟的一点：报错点离操作点很远。** 用户在 `xlings use` 之后半小时才在编译时炸掉，几乎不可能
把两件事联系起来，于是去怀疑自己的代码 / CMake / 环境变量。

#### 只落地本 PR（当前 Draft）

程序侧会好一截：注册时写入了 root↔member 星形边（`registration.cppm:777-786`），
`cmd_use` 的遍历现在能从 `g++` 走到 root 再走到全部成员，
**"gcc 换了 g++ 没换"这个具体症状大概率消失**。

但整体体验**净变差**，因为 §3.1：

```console
$ xlings install gcc@15.1.0
$ xlings use gcc 15.1.0
✓ gcc -> 15.1.0
$ gcc --version && g++ --version
gcc 15.1.0
g++ 15.1.0                       # ✓ 程序对齐了

$ ls ~/.xlings/subos/.../usr/include/c++/
16.1.0                           # ← 头文件根本没换，还是上次装的
```

从「程序错乱、头文件半错」变成「程序对齐、头文件完全不换」。**故障更隐蔽了** —— 因为
`gcc --version` / `g++ --version` 这两个用户唯一会检查的命令现在都是对的。

> 这就是把 §3.1 列为合并阻塞项的原因：它让一个"检查一下能发现"的问题变成"检查了也发现不了"。

#### 全部完成后

```console
$ xlings use gcc 15.1.0
✓ gcc       -> 15.1.0
✓ g++       -> 15.1.0
✓ gcc-ar    -> gcc-15.1.0
✓ libstdc++ -> gcc-15.1.0
✓ headers   -> gcc-15.1.0/include
```

或者，**什么都不发生**：

```console
$ xlings use gcc 15.1.0
✗ cannot switch to gcc@15.1.0
  reason:   group member 'libstdc++@gcc-15.1.0' is missing from the version database
  provider: pkgindex:gcc@15.1.0
  hint:     xlings install gcc@15.1.0 --repair
  nothing was changed
```

**关键区别不是"成功了"，是没有第三种结果。**

### 5.2 场景二：卸载一个版本

#### 修复前

```console
$ xlings remove gcc@16.1.0
✓ removed
$ g++ --version
bash: g++: command not found      # ← 15 的 g++ 也没了
```

原因：recipe 对 `g++`、虚拟 root、`cc` 发的是**无版本 remove**，`origin/main:remove_version()`
在 bare version 下命中所有 `ns:version` 匹配项；旧的 `script.cppm` / `subos.cppm` 路径甚至直接
`versions_mut().erase(name)`。**删一个 release，误伤同名目标的全部 release。** 残留反向边不会
被清理，下一次 `use` 会踩上去。

#### 全部完成后

```console
$ xlings remove gcc@16.1.0
✓ removed gcc@16.1.0 (5 members)
✓ active group switched to gcc@15.1.0
$ g++ --version
g++ 15.1.0
```

⚠️ 但按**当前实现**（§3.3）可能是：

```console
$ g++ --version
g++ 15.1.0 (musl)     # ← 来自 musl provider，和 gcc 15 不是一套
```

**这正是本 PR 想消灭的那类状态，只是从"切换时产生"挪到了"删除时产生"。**

### 5.3 场景三：状态已经坏了之后

| | 修复前 | 完成后 |
|---|---|---|
| 用户怎么发现 | 编译报错，反推半天 | `xlings use` 当场拒绝并说明 |
| 诊断信息 | 无 | provider / target / version / JSON Pointer 路径 |
| 自救手段 | 手改 `versions.json`，或删 `~/.xlings` 重装 | `xlings self doctor --fix`（**Task 6，尚未落地**） |
| 会不会越修越坏 | 会（重装只重建一部分，残留边还在） | 不会（注册是全量校验后一次应用） |

**中间态的坑** —— 本 PR 已落地 fail-closed 拒绝，但 doctor 还不认识 binding 元数据，且
`bindingIntegrityIssues` 会被写回磁盘固化：

```console
$ xlings use gcc 15.1.0
✗ binding metadata integrity issue 'binding-group-field-invalid'
  at '/bindingGroup/rootVersion'

$ xlings self doctor --fix
✓ workspace/shim consistency OK      # ← 它看不见这个问题

$ # 然后呢？
```

**死路。** 这就是"强制校验不应先于恢复手段发布"的具体含义 —— 不是理论洁癖，是用户真的会卡住。

### 5.4 必须提前准备的体验倒退

fail-closed 会把一批**过去静默通过**的操作变成明确失败。用户的第一感受是"升级之后我原来能装的
包装不上了"：

```console
$ xlings install some-lib
✗ install failed
```

来源有两处：§3.2 的 `HeaderAmbiguous`、§3.7 的 `OwnershipConflict`。当前两者都只走
`log::warn` + `return false`，用户在默认日志级别下**只看到 "install failed"，看不到原因**。

这类"从静默错误变成明确报错"的转变本身是对的，但必须配三样东西，缺一样用户就会认为是 xlings
坏了而不是自己的包坏了：

1. 用户级（非 debug 级）错误文案，说明**是什么冲突**
2. 一条可执行的出口（`--force` / 迁移命令 / 明确的"请升级该 recipe"）
3. 发布前对 pkgindex 做一遍审计（Task 9）

### 5.5 汇总表

| 用户动作 | 修复前 | 只落地本 PR | 全部完成 |
|---|---|---|---|
| `use` 切换程序 | 静默错乱 | ✅ 对齐 | ✅ 对齐 |
| `use` 切换头文件/库 | 半对(只换入口 target) | ❌ **完全不换** | ✅ 整组换 |
| `use` 失败时 | 静默半切 | 静默半切 | ✅ 原样不动 + 具名原因 |
| `remove` 一个 release | 误删同名全部版本 | ✅ 精确删除 | ✅ 精确删除 |
| `remove` 后的回退 | 残留悬空边 | ⚠️ 可能跨 provider 混搭 | ✅ 整组一致 |
| 坏状态可发现性 | 极差(延迟到编译期) | 中(当场拒绝) | ✅ 当场拒绝 |
| 坏状态可恢复性 | 无 | ❌ **无(死路)** | ✅ `doctor --fix` |
| 安装成功率 | 高(但可能装坏) | ⚠️ 部分 recipe 硬失败 | ✅ 索引审计后恢复 |

**核心价值主张**：这套改动真正卖给用户的不是"切换更快"或"功能更多"，而是
**把一个会静默产生错误状态、且错误在几十分钟后才以无关面貌爆发的系统，换成一个要么正确、
要么当场拒绝的系统**。

而这个价值主张只有在 fail-closed **和**恢复通道同时到位时才成立。只有前者的话，用户拿到的是
"以前能凑合用，现在直接不让我用，还没法修" —— 那会被理解为倒退，尽管技术上更正确。

---

## 6. 建议（按优先级）

> **合并目标已变更**：依 plan 决议 Q5，本 PR 的 base 应从 `main` 改为
> **`release/0.4.70` 集成分支**。下文的"合并"均指合入该集成分支；对外发布由集成分支
> 统一验证后一次性合入 main 完成（见 `2026-07-26-xvm-group-transaction-plan.md` §0.1、R）。
>
> 这也放宽了一项约束：**过渡性实现（如 P0.1 的 `includedir` 临时双写）是可接受的** ——
> 它不会进入 main 的任何提交，会在 R.1 阶段直接删除。

### 合并（进集成分支）前必须做

1. 修 §3.1 —— 要么恢复写 `includedir`/`libdir`，要么把 `bindingHeaders` 的消费者一起并进来
2. 修 §3.2 —— header 默认归属批次主节点所在 group，而非直接 `HeaderAmbiguous`
3. 修 §3.3 —— 回退改保守语义：凑不齐完整同 group 选择就整组停用
4. 修 §3.7 —— `OwnershipConflict` / `HeaderAmbiguous` 给用户级错误 + 出口
5. PR body 把 "atomically" 降级为准确措辞

### 合并前强烈建议

6. 把 Task 6（binding 诊断 + `self doctor --fix`）提到与本 PR 同批
7. `platform::write_file_atomic()`（temp + fsync + rename）—— 10 行代码，修的是比 binding
   group 更严重的问题
8. per-home advisory lock —— 保护 registration/removal，不必等 Task 4

### 后续

9. 把 group 提升为顶层表（详见 `2026-07-26-xvm-group-transaction-design.md` §2）
10. `bindingIntegrityIssues` 不落盘 —— 它是解析期派生数据
11. 拆 PR：`db/types` + 纯函数 resolver | registration | removal | installer adapter
12. `test_main.cpp` 按子系统拆分；对不变量集合上 property-based 测试

---

## 7. 最终判断

架构方向**支持**，且高于"能用就行"的水准 —— 生态审计、备选方案、失败分类、Draft 边界声明都
可以拿出去当范例。

但这版提交**当前不可合并**：两个静默回归会让用户在切版本时才发现坏了，而 fail-closed 又还
没有配套的自救通道。修掉建议 1–5、把 6–8 提前，并把 base 改为 `release/0.4.70`，
这就是一个合格的基础层。

**注意**：改用集成分支模型后，"不可合并"的判定标准也随之改变 —— 判据不再是"能否直接面对
用户"，而是"**是否引入不可逆的倒退，或阻塞后续批次**"。§3.1 / §3.2 两条仍然是硬阻塞
（前者会让 P4.2 的 E2E 无法建立基线，后者会阻塞 P4.1 的索引审计）；而 §3.5 的措辞问题、
§3.8 的性能备注可以留到 R 阶段一并处理。

后续目标架构与实施路径见：

- `2026-07-26-xvm-group-transaction-design.md`
- `2026-07-26-xvm-group-transaction-plan.md`
