XingfenD/crearte-monorepo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(spec): close the P15-A branch review's N1-N4 staleness in the A spec

Browse Source 
The full-branch review of chore/p15a-approve-phases returned APPROVE with
notes: the merged branch code itself has zero defects, but four places in this
spec disagreed with the code it describes. All four came from the controller
writing executable details from memory during the first re-pin.

N1 - applyApprovalInTx parameter order, three places (D-C row, the §3.1
skeleton call site, the §3.2 phase-six signature). The spec said
`plan approvePlan, submissionID`; the code has always been
`submissionID string, plan approvePlan`. The implementer's order is legal and
better: a scalar identifier before an aggregate matches Go convention.

N2 - §3.2 migration ranges the first re-pin claimed to have fixed but had not:
48-85 -> 48-86 (86 is the closing brace of the coverUpload block) and
87-100 -> 88-101 (87 is a blank line, 88 is `switch sub.Kind`).

N3 - the §1.2 difference-table header: phase four 87-100 -> 88-101, phase six
136-186 -> 136-199 (186 is a mid-branch line inside MetadataChange; 199 is the
closing brace of the switch).

N4 - §T1.4 said the CG2 needle's only hit is `:196`, which is the pre-F9 line
number from dda3fe8; HEAD measures `:201`. The (b) blind-spot note said
"126-200 = 77 lines"; 126-200 is 75 lines - 77 belongs to the §1.1 range
125-201 and was carried over by mistake.

Every corrected value was re-derived from the base tree fe810dd and HEAD
b15c2a8 line by line rather than copied from the review, and the two
derivations agree. A RE-PIN note at §3.2 phase six records the true values and
the lesson, which §8 discipline 11 now states as a rule: executable details in
a spec (regexes, signatures, literals, line ranges) must be grepped out of the
code entity and pasted, never hand-written.

Post-write checks: 20/20 assertions, table column counts unchanged (12 blocks,
0 anomalies), 8 code fences paired, 28 headings with no duplicates.

Closes N1-N4 of crearte-server/.superpowers/sdd-p15a/final-review-report.md.
N5 (the verification artifact's fake rigor) is closed separately in the
gitignored evidence area: verify-fix-v8.py now runs 106 real checks with zero
bare prints, and the v2 output it cites is archived on disk.
This commit is contained in:
XingfenD
2026-10-04 02:00:45 +08:00
parent 02188eb3e0
commit 37a0a8ddd1
1 changed files with 10 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/specs/2026-10-03-p15a-approve-phases-design.md
+10 -8
View File
@@ -50,7 +50,7 @@
两者都 `switch sub.Kind`、都查 `Works().GetByID` / `Versions().Get`、都可能返回 `ErrWorkIDTaken`/`ErrVersionExists`，故看上去可以抽成一个"重复性检查" helper 共用。**实测比对后确认不可合并**，四处实质差异： 两者都 `switch sub.Kind`、都查 `Works().GetByID` / `Versions().Get`、都可能返回 `ErrWorkIDTaken`/`ErrVersionExists`，故看上去可以抽成一个"重复性检查" helper 共用。**实测比对后确认不可合并**，四处实质差异：
| 差异 | 阶段④（87–100） | 阶段⑥（136–186） | | 差异 | 阶段④（88–101） | 阶段⑥（136–199） |
|---|---|---| |---|---|---|
| **kind 覆盖** | 只有 `NewWork` / `NewVersion` 两个 case | 三个 case：`NewWork` / `NewVersion` / **`MetadataChange`** | | **kind 覆盖** | 只有 `NewWork` / `NewVersion` 两个 case | 三个 case：`NewWork` / `NewVersion` / **`MetadataChange`** |
| **`NewVersion` 的额外校验** | 只查 `Versions().Get` 是否已存在 | **还查** `work.Runtime != model.RuntimeVirtual \|\| bundleUpload == nil` → `ErrSubmissionConflict`；且 `Versions().Create` 后**还调** `tx.Works().Touch` | | **`NewVersion` 的额外校验** | 只查 `Versions().Get` 是否已存在 | **还查** `work.Runtime != model.RuntimeVirtual \|\| bundleUpload == nil` → `ErrSubmissionConflict`；且 `Versions().Create` 后**还调** `tx.Works().Touch` |
@@ -138,7 +138,7 @@ func ParseSubmissionEnvelope(raw []byte) (SubmissionEnvelope, error) // 值返
|---|---|---|---| |---|---|---|---|
| **D-A** | 拆分粒度 | **按七阶段拆 5 个 helper**（①②③ 合为一个"输入装载"、④ 一个、⑤ 一个、⑥ 一个（内部再拆 3 个 kind 分支）、⑦ 一个），不逐行拆 | §1.1 的阶段边界是**语义边界**（副作用发生点），不是任意切点。①②③ 都只读、都产出 plan 的字段、失败时零副作用 → 合为一个 helper 不损失可读性；④⑤⑥⑦ 各自有独立副作用（预检/Copy/事务/清理）→ 必须分开 | | **D-A** | 拆分粒度 | **按七阶段拆 5 个 helper**（①②③ 合为一个"输入装载"、④ 一个、⑤ 一个、⑥ 一个（内部再拆 3 个 kind 分支）、⑦ 一个），不逐行拆 | §1.1 的阶段边界是**语义边界**（副作用发生点），不是任意切点。①②③ 都只读、都产出 plan 的字段、失败时零副作用 → 合为一个 helper 不损失可读性；④⑤⑥⑦ 各自有独立副作用（预检/Copy/事务/清理）→ 必须分开 |
| **D-B** | plan 结构体 | 新增**包级私有 struct** `approvePlan`，字段：`sub model.Submission`、`p WorkPayload`、`owner model.User`、`bundleUpload *model.Upload`、`coverUpload *model.Upload`、`finalBundleKey string`、`finalCoverKey string` | 七阶段之间传递的就是这七样东西（实测自函数体）。用 struct 而非 7 个返回值：Go 的多返回值超过 4 个即难读，且 ⑤ 要往 plan 里写 `finalBundleKey`/`finalCoverKey` 供 ⑥⑦ 用 | | **D-B** | plan 结构体 | 新增**包级私有 struct** `approvePlan`，字段：`sub model.Submission`、`p WorkPayload`、`owner model.User`、`bundleUpload *model.Upload`、`coverUpload *model.Upload`、`finalBundleKey string`、`finalCoverKey string` | 七阶段之间传递的就是这七样东西（实测自函数体）。用 struct 而非 7 个返回值：Go 的多返回值超过 4 个即难读，且 ⑤ 要往 plan 里写 `finalBundleKey`/`finalCoverKey` 供 ⑥⑦ 用 |
| **D-C** | helper 形态 | ①②③④⑤⑦ → **未导出方法**（需 `s.store`/`s.users`/`s.objects`）；⑥ 事务体 → **包级函数** `applyApprovalInTx(ctx, tx repository.ContentRepository, plan approvePlan, submissionID, adminID, note string) error`；⑥ 内三个 kind 分支 → **包级函数** | §1.3 实测两种形态都不触守卫；选择依据是"是否需要接收者字段"。事务体不碰 `s.*`（只用 `tx`）→ 包级函数更纯，且签名自证"事务内不得访问 store 外的东西"。体例分别同 `deleteAccountLocked` 与 `versionFromUpload` | | **D-C** | helper 形态 | ①②③④⑤⑦ → **未导出方法**（需 `s.store`/`s.users`/`s.objects`）；⑥ 事务体 → **包级函数** `applyApprovalInTx(ctx, tx repository.ContentRepository, submissionID string, plan approvePlan, adminID, note string) error`；⑥ 内三个 kind 分支 → **包级函数** | §1.3 实测两种形态都不触守卫；选择依据是"是否需要接收者字段"。事务体不碰 `s.*`（只用 `tx`）→ 包级函数更纯，且签名自证"事务内不得访问 store 外的东西"。体例分别同 `deleteAccountLocked` 与 `versionFromUpload` |
| **D-D** | ④与⑥**不合并** | 各自独立 helper，不共享代码 | §1.2：四处实质差异 + 乐观/悲观语义不同 + 错误文案不同。表面重复是**有意的 TOCTOU 双层校验** | | **D-D** | ④与⑥**不合并** | 各自独立 helper，不共享代码 | §1.2：四处实质差异 + 乐观/悲观语义不同 + 错误文案不同。表面重复是**有意的 TOCTOU 双层校验** |
| **D-E** | 事务边界与 Copy 顺序 | **一行不动**：⑤ 的两次 `s.objects.Copy` 必须在 ⑥ `WithTx` **之外**（故 ⑦ 需要清理）；⑥ 的全部写入必须在 `WithTx` **之内**；`MarkReviewed` 必须是事务内**最后一个**调用 | 改变顺序即行为变更：Copy 进事务会让长耗时 I/O 持锁；`MarkReviewed` 提前会让"实体建成但状态未改"的中间态可被并发观察到 | | **D-E** | 事务边界与 Copy 顺序 | **一行不动**：⑤ 的两次 `s.objects.Copy` 必须在 ⑥ `WithTx` **之外**（故 ⑦ 需要清理）；⑥ 的全部写入必须在 `WithTx` **之内**；`MarkReviewed` 必须是事务内**最后一个**调用 | 改变顺序即行为变更：Copy 进事务会让长耗时 I/O 持锁；`MarkReviewed` 提前会让"实体建成但状态未改"的中间态可被并发观察到 |
| **D-F** | `Features` 三态语义 | `if p.Features == nil { work.Features = existing.Features }` **连注释一起搬**进 MetadataChange helper，**不得"简化"** | 该注释原文：「旧提交（features 采集上线前创建）的 payload 无 features 键：保留现值，避免审批通过时静默清空已发布作品的运行权限；显式 `{}` 才是清空」。三态（nil=保留 / 空 map=清空 / 有值=覆盖）由 `content_test.go:237` 精确钉住，简化即红 | | **D-F** | `Features` 三态语义 | `if p.Features == nil { work.Features = existing.Features }` **连注释一起搬**进 MetadataChange helper，**不得"简化"** | 该注释原文：「旧提交（features 采集上线前创建）的 payload 无 features 键：保留现值，避免审批通过时静默清空已发布作品的运行权限；显式 `{}` 才是清空」。三态（nil=保留 / 空 map=清空 / 有值=覆盖）由 `content_test.go:237` 精确钉住，简化即红 |
@@ -176,7 +176,7 @@ func (s *ModerationService) Approve(ctx context.Context, adminID, submissionID,
return err return err
} }
err = s.store.WithTx(ctx, func(tx repository.ContentRepository) error { err = s.store.WithTx(ctx, func(tx repository.ContentRepository) error {
return applyApprovalInTx(ctx, tx, plan, submissionID, adminID, note) // ⑥ return applyApprovalInTx(ctx, tx, submissionID, plan, adminID, note) // ⑥
}) })
if err != nil { if err != nil {
if errors.Is(err, repository.ErrConflict) { if errors.Is(err, repository.ErrConflict) {
@@ -195,13 +195,13 @@ func (s *ModerationService) Approve(ctx context.Context, adminID, submissionID,
### 3.2 逐阶段要求 ### 3.2 逐阶段要求
**①②③ `loadApprovePlan`**（未导出方法）： **①②③ `loadApprovePlan`**（未导出方法）：
- 逐字搬 48–85 行。返回 `(approvePlan, error)`。 - 逐字搬 **48–86** 行。返回 `(approvePlan, error)`。
- 三处错误映射逐字保留：`errors.Is(err, repository.ErrNotFound)` → `ErrContentNotFound`；`sub.Status != model.SubmissionStatusPending` → `ErrSubmissionConflict`；`fmt.Errorf("service: load submission: %w", err)`；`fmt.Errorf("service: resolve submitter: %w", err)`；两处 `ErrUploadUnavailable`（含 `up.ConsumedBy != sub.ID` 判定）。 - 三处错误映射逐字保留：`errors.Is(err, repository.ErrNotFound)` → `ErrContentNotFound`；`sub.Status != model.SubmissionStatusPending` → `ErrSubmissionConflict`；`fmt.Errorf("service: load submission: %w", err)`；`fmt.Errorf("service: resolve submitter: %w", err)`；两处 `ErrUploadUnavailable`（含 `up.ConsumedBy != sub.ID` 判定）。
- ② 的 `if sub.Kind == model.SubmissionKindNewWork` 条件**必须保留**（其它 kind 不查 owner，`plan.owner` 为零值；⑥ 的 `NewWork` 分支才用 `owner.ID`）。 - ② 的 `if sub.Kind == model.SubmissionKindNewWork` 条件**必须保留**（其它 kind 不查 owner，`plan.owner` 为零值；⑥ 的 `NewWork` 分支才用 `owner.ID`）。
- ③ 的 `bundleUpload = &up` 取地址语义**必须保留**（`up` 是循环内局部变量，取其地址是本函数刻意的写法；改成存值会让 `plan.bundleUpload != nil` 判定失效）。 - ③ 的 `bundleUpload = &up` 取地址语义**必须保留**（`up` 是循环内局部变量，取其地址是本函数刻意的写法；改成存值会让 `plan.bundleUpload != nil` 判定失效）。
**④ `precheckApprove`**（未导出方法）： **④ `precheckApprove`**（未导出方法）：
- 逐字搬 87–100 行。**保持用 `s.store`**（不是 `tx`）。 - 逐字搬 **88–101** 行。**保持用 `s.store`**（不是 `tx`）。
- 错误文案 `fmt.Errorf("service: approve precheck: %w", err)` **逐字保留**（两处）。 - 错误文案 `fmt.Errorf("service: approve precheck: %w", err)` **逐字保留**（两处）。
- **不得**增加 `MetadataChange` case（④ 现在没有，加了会改变 `MetadataChange` 提交的失败时机）。 - **不得**增加 `MetadataChange` case（④ 现在没有，加了会改变 `MetadataChange` 提交的失败时机）。
@@ -211,7 +211,9 @@ func (s *ModerationService) Approve(ctx context.Context, adminID, submissionID,
- `storage.ErrObjectNotFound` → `ErrUploadUnavailable` 的映射逐字保留；`service: copy bundle|cover: %w` 两条文案逐字保留。 - `storage.ErrObjectNotFound` → `ErrUploadUnavailable` 的映射逐字保留；`service: copy bundle|cover: %w` 两条文案逐字保留。
**⑥ `applyApprovalInTx`**（包级函数）： **⑥ `applyApprovalInTx`**（包级函数）：
- 搬 **126–200** 行（`WithTx` 闭包体）。⚠️ **`:201` 是 `})`，`:202–207` 的事务错误映射属阶段⑦，都不得搬进 helper**（照抄旧区间「126–205」会把 `})` 与闭包外半截搬进去 → 编译红或行为变更）。签名收 `(ctx, tx repository.ContentRepository, plan approvePlan, submissionID, adminID, note string) error`（**含 `submissionID`，见 F9 裁定**）。
> ⚠️ **RE-PIN 2026-10-04（第二次，闭合全分支终审 N1–N4）**：本 spec 里凡可执行的技术细节（形参顺序、行号区间、needle 命中行）一律以**代码实体**为准 —— `applyApprovalInTx` 的形参序是 `ctx context.Context, tx repository.ContentRepository, submissionID string, plan approvePlan, adminID, note string`（`submissionID` 在 `plan` 之前）；base 树 `fe810dd` 的阶段区间为 ①②③`48–86` / ④`88–101` / ⑥闭包体`126–200`（75 行）/ ⑥整体`125–201`（77 行）/ ⑥内 switch`136–199`；HEAD 上 CG2 needle 唯一命中 `:201`。首次 re-pin 时控制者凭记忆写了 4 处不一致（N1 形参序 3 处、N2/N3 区间 staleness、N4 行号与算术），全分支终审逐条实测抓出。**教训已入 §8 纪律 11：可执行细节必须从代码实体 grep 出来粘贴，不得手写。**
- 搬 **126–200** 行（`WithTx` 闭包体）。⚠️ **`:201` 是 `})`，`:202–207` 的事务错误映射属阶段⑦，都不得搬进 helper**（照抄旧区间「126–205」会把 `})` 与闭包外半截搬进去 → 编译红或行为变更）。签名收 `(ctx, tx repository.ContentRepository, submissionID string, plan approvePlan, adminID, note string) error`（**含 `submissionID`，见 F9 裁定**）。
- **`GetByIDForUpdate` 锁行 + 重校 `Status == Pending` 必须在最前**（125–134），且 `errors.Is(err, repository.ErrNotFound)` → `ErrSubmissionConflict`（**注意与①不同**：① 映射 `ErrContentNotFound`，⑥ 映射 `ErrSubmissionConflict`——这个差异是刻意的，事务内消失意味着并发删除，语义是冲突而非未找到）。 - **`GetByIDForUpdate` 锁行 + 重校 `Status == Pending` 必须在最前**（125–134），且 `errors.Is(err, repository.ErrNotFound)` → `ErrSubmissionConflict`（**注意与①不同**：① 映射 `ErrContentNotFound`，⑥ 映射 `ErrSubmissionConflict`——这个差异是刻意的，事务内消失意味着并发删除，语义是冲突而非未找到）。
- 三个 kind 分支各自拆包级函数（`createWorkInTx` / `createVersionInTx` / `applyMetadataChangeInTx`），或保留为一个 switch——**由实现者按可读性定**，但 `switch` 的分支顺序与 `MarkReviewed` 在末尾的位置**不得变**。 - 三个 kind 分支各自拆包级函数（`createWorkInTx` / `createVersionInTx` / `applyMetadataChangeInTx`），或保留为一个 switch——**由实现者按可读性定**，但 `switch` 的分支顺序与 `MarkReviewed` 在末尾的位置**不得变**。
- `MetadataChange` 分支的 `Features` 三态**连注释一起搬**（D-F）。 - `MetadataChange` 分支的 `Features` 三态**连注释一起搬**（D-F）。
@@ -265,7 +267,7 @@ P14 的七条断言钉的是**类型形态**，没有一条钉**函数长度**
**mutation 自查（实现者必做并附输出，每条测"预期红 + 其余绿 + 非编译红 + 恢复证明"）**： **mutation 自查（实现者必做并附输出，每条测"预期红 + 其余绿 + 非编译红 + 恢复证明"）**：
- (a) 把 `Approve` 的某阶段**内联回去**（helper 存在但 `Approve` 不调它，改为原地展开）→ `TestApproveIsSmall` 必须红 - (a) 把 `Approve` 的某阶段**内联回去**（helper 存在但 `Approve` 不调它，改为原地展开）→ `TestApproveIsSmall` 必须红
- (b) 给某个 helper 塞代码使 `Approve` 仍 <60 但**某函数 >100**（例如把⑥的三个 kind 分支全塞进 `applyApprovalInTx` 而不拆）→ `TestNoOversizedFunction` 必须红 - (b) 给某个 helper 塞代码使 `Approve` 仍 <60 但**某函数 >100**（例如把⑥的三个 kind 分支全塞进 `applyApprovalInTx` 而不拆）→ `TestNoOversizedFunction` 必须红
> ⚠️ **(b) 的可行性须先实测**：若⑥整体（126–200 = **77 行**）搬进 `applyApprovalInTx` 而不再拆 → **不会红**；**更宽的实测盲区：仅合并④⑥（正是 §2.1 被否方案 A 的形态）= 91 行，仍不红**；真红下界实测 >100 行。这是**阈值 100 的已知盲区**，不是守卫失效：它钉的是"不再有 169 行的巨函数"，不是"每个 helper 都短"。实现者须在报告里写明这一点，**不得声称 (b) 覆盖了所有塞代码形态**。若要在 (b) 上取得真红，须构造 >100 行的 helper（例如把 ④⑥ 两个 switch 都塞进一个 helper）。 > ⚠️ **(b) 的可行性须先实测**：若⑥整体（126–200 = **75 行**）搬进 `applyApprovalInTx` 而不再拆 → **不会红**；**更宽的实测盲区：仅合并④⑥（正是 §2.1 被否方案 A 的形态）= 91 行，仍不红**；真红下界实测 >100 行。这是**阈值 100 的已知盲区**，不是守卫失效：它钉的是"不再有 169 行的巨函数"，不是"每个 helper 都短"。实现者须在报告里写明这一点，**不得声称 (b) 覆盖了所有塞代码形态**。若要在 (b) 上取得真红，须构造 >100 行的 helper（例如把 ④⑥ 两个 switch 都塞进一个 helper）。
- (c) 删掉一个 helper（把其内容合并进 `Approve`）→ `TestApproveHelpersExist` 必须红 - (c) 删掉一个 helper（把其内容合并进 `Approve`）→ `TestApproveHelpersExist` 必须红
- (d) 把 helper 名改成别的（如 `loadApproveInputs`）→ `TestApproveHelpersExist` 必须红（**这条是 (c) 的对照**：证明断言钉的是名字集合而非"存在任意 5 个函数"） - (d) 把 helper 名改成别的（如 `loadApproveInputs`）→ `TestApproveHelpersExist` 必须红（**这条是 (c) 的对照**：证明断言钉的是名字集合而非"存在任意 5 个函数"）
- (e) 把 `TestNoOversizedFunction` 的扫描范围改成一个空目录 → 必须红（**防"扫 0 文件也报 0 违规"的假信心**，P13 审查者 M5 的同族教训；断言内须含"至少扫到 N 个 `.go` 文件"的前提检查） - (e) 把 `TestNoOversizedFunction` 的扫描范围改成一个空目录 → 必须红（**防"扫 0 文件也报 0 违规"的假信心**，P13 审查者 M5 的同族教训；断言内须含"至少扫到 N 个 `.go` 文件"的前提检查）
@@ -284,7 +286,7 @@ spec §7 风险表第 1 行原写「T3 字节级比对会暴露合并」——**
- **CG1**：`fmt.Errorf("service: approve precheck: %w"` 只能出现在 `precheckApprove` 的 span 内 - **CG1**：`fmt.Errorf("service: approve precheck: %w"` 只能出现在 `precheckApprove` 的 span 内
- **CG2**：`.Submissions().GetByIDForUpdate(` 只能出现在 `applyApprovalInTx` 的 span 内 - **CG2**：`.Submissions().GetByIDForUpdate(` 只能出现在 `applyApprovalInTx` 的 span 内
> ⚠️ needle **必须收紧成 `.Submissions().GetByIDForUpdate(`**：`moderation.go` 内唯一命中 `:196`；而 `account.go:46`、`auth.go:122` 是 **`users.GetByIDForUpdate`**（不同接收者）。不收紧则将来 `moderation.go` 新增别的 `GetByIDForUpdate` 调用会误红。 > ⚠️ needle **必须收紧成 `.Submissions().GetByIDForUpdate(`**：`moderation.go` 内唯一命中 `:201`；而 `account.go:46`、`auth.go:122` 是 **`users.GetByIDForUpdate`**（不同接收者）。不收紧则将来 `moderation.go` 新增别的 `GetByIDForUpdate` 调用会误红。
- **CG3**：两个 helper 体内各自含 `switch …Kind`，且**互不调用** - **CG3**：两个 helper 体内各自含 `switch …Kind`，且**互不调用**
**关键设计**：语料**先过 `stripComments`（只掩注释、保留字符串字面量）**。`moderation.go:141` 与 `:191` 的注释里**合法地**提到 precheck 文案 → 不掩注释会假红；而字符串字面量必须保留，否则 `fmt.Errorf("...")` 这个 needle 根本匹配不到。span 复用 `function_length_test.go` 的 `scanFuncs`（共享同一分词器，也就共享 F2 的闭包盲区，但闭包形态与 ④⑥ 合并无关）。 **关键设计**：语料**先过 `stripComments`（只掩注释、保留字符串字面量）**。`moderation.go:141` 与 `:191` 的注释里**合法地**提到 precheck 文案 → 不掩注释会假红；而字符串字面量必须保留，否则 `fmt.Errorf("...")` 这个 needle 根本匹配不到。span 复用 `function_length_test.go` 的 `scanFuncs`（共享同一分词器，也就共享 F2 的闭包盲区，但闭包形态与 ④⑥ 合并无关）。