diff --git a/.task/reviews/2026-10-19-permux-independent-review.md b/.task/reviews/2026-10-19-permux-independent-review.md new file mode 100644 index 00000000..56822e49 --- /dev/null +++ b/.task/reviews/2026-10-19-permux-independent-review.md @@ -0,0 +1,130 @@ +# permux 独立代码审查（Independent Review）— 2026-10-19 + +> 审查者：全新会话（未参与实现），按 `.task/feature-iam-role-permission-linked-capabilities-perm-ux.md` 与本审查任务卡执行独立 Read-Only Review。 +> 审查对象：Dev 子分支 `feature/iam-role-permission-linked-capabilities-perm-ux`（HEAD 234ce30；起始基线 90317d02）；组件 commit `modules/backend @ 4130ad78 = origin/feature/iam-perm-ux-backend`，`modules/frontend @ 46a63f02 = origin/feature/iam-perm-ux-frontend`。 +> 终态：组合 pin `modules/backend @ 4130ad7 + modules/frontend @ 46a63f0`，6/6 单元（U1–U5 + U6 组合交付）落 Dev 子分支。 +> 审查范围：组件仓 `origin/main…origin/feature/iam-perm-ux-*` 与 Dev 父仓 `90317d02..HEAD` 的提交范围；只读分析与命令验证，不修改、不暂存、不提交。 + +## 结论 + +**结论：PASS（附 1 项非阻塞观察、4 项非阻塞建议）**。本任务卡记录的实现范围（permux-U1–U6）符合： +- 数据完整性：批量事务原子性、幂等回执、单 epoch、锁序一致； +- 验收一致性：C01–C04 / C07–C10 在真实 Backend + 真实 Frontend dist 浏览器活体（8 场景）PASS；C05 / C06 / C11 / C12 / C15 由 store/HTTP 单测覆盖且测试通过；C13 由 L1 U7 交付；C14 由 design-guard + 实现约定 + 窄屏 helper 覆盖； +- 权限边界：新增端点门禁与既有 `requireSystemAction` 契约一致；只读预览无副作用； +- 设计系统：design-guard 棘轮 7/7 通过（见验证命令 §6.4）； +- 测试充分性：阻塞项为零；下列非阻塞项不影响目标交付。 + +--- + +## 验证命令与结果（仅读、未写） + +| 命令 | 目的 | 结果 | +|---|---|---| +| `GOWORK=off go test -count=1 ./internal/authz/... ./internal/store ./internal/server -run "Role\|Policy\|Binding\|AccountSearch\|Batch\|CopyAssign\|Members\|Preview\|Catalog\|System"` | Backend 角色/权限/分配/预览/复制/成员/目录 store+server 单测（覆盖 permux-U1/U2/U6 范围与 409/幂等/版本/批上限/复制联动负控） | `ok agent-system-backend/internal/authz/catalog`、`ok agent-system-backend/internal/store 1.325s`、`ok agent-system-backend/internal/server 3.737s` | +| `npx vitest run test/design-guard.test.ts` | DESIGN.md §11 棘轮（动效唯一所有者、裸色值、行内字号、`
`） | 7/7 PASS，0.127s | +| `npx vitest run test/role-permissions.test.ts test/role-permissions-ui.test.ts test/role-members.test.ts test/permission-explanations.test.ts test/account-users-panel.test.ts test/design-guard.test.ts` | Frontend permux-U3/U4/U5 单元测试 + 跨页选择/重放/草稿基线/成员过滤/设计守护 | 53/53 PASS，0.460s | + +注：`make test` 全量未跑（任务卡「范围纪律」要求）；上述过滤后直接检查与实现范围对齐。 + +--- + +## 重点审查逐项 + +### 重点 1 — Backend 批量事务原子性与锁序 + +| 子项 | 结论 | 依据 | +|---|---|---| +| 批量事务原子性（PatchRolePermissionsBatch / AssignUserRolesBatch / CreateRoleCopyAndAssign） | PASS | 三个批量写共用 `policyWriteBatch`，事务内任意错误 `defer tx.Rollback()` 回滚；`PatchRolePermissionsBatch` 通过循环 `patchRolePermissionsTx` 后再统一 `UPDATE authority_state` 推进 epoch，单一幂等回执（`role_policy.go:1078-1095`、`1087-1095`）；`AssignUserRolesBatch` 先遍历所有 item 完成 `checkRoleAssignmentTx` 预校验，再 rebind，最后 `checkCapabilityAssociationTx` + `UPDATE authority_state`（`role_policy.go:1078-1095`、`1086-1109`）；`CreateRoleCopyAndAssign` 三步（source 校验 → patch 插入 → assign 事务）共用同一 tx（`role_policy.go:1335-1398`）。负控：`TestBatchPatchRolePermissionsStaleLimitAndIdempotency` 验证陈旧版本整批不写、批上限 21、角色重复；`TestBatchAssignUserRolesRejectsWholeBatch` 验证陈旧成员/停用用户/缺失用户/停用角色/重复用户/101 人都整批拒绝；`TestCopyAssignRejectsWithoutResidue` 验证名称重复/源陈旧/用户陈旧/无效 override/101 人/缺 `roles.assign` 全部 0 残留；`TestCopyAssignCreatesRoleAndBindsAtomically` 验证成功路径源角色不变、绑定生效、幂等回执。 | +| 单次 epoch 推进 | PASS | 所有写路径共用 `UPDATE authority_state SET authority_epoch=epoch+1 WHERE authority_epoch=epoch`，陈旧时 `RowsAffected()!=1` 返回 `ErrRolePolicyVersion` / `ErrConflict`。`TestBatchAssignUserRolesRebindsAndReplays` 断言 `batch.AuthorityEpoch==epoch+1` 且重放不再推进（`role_policy_test.go:1107-1115`）。 | +| 幂等回执（同键异 payload 拒绝） | PASS | `replayReceipt` 比较 `operation` + `requestHash`，同键异 payload 返回 `ErrConflict`（`role_policy.go:435-455`）。`TestRolePolicyIdempotencyKeyCannotChangeRequest` 验证 `ReplaceRolePermissions`；`TestBatchAssignUserRolesRebindsAndReplays` 验证批路径；`TestBatchPatchRolePermissionsStaleLimitAndIdempotency` 验证 `batch_patch`。 | +| policyWriteBatch 锁序（用户 → authority_state → 角色行） | PASS | `policyWriteBatch` 先 `lockAccountUsers(tx, owners...)`（owner 排序），再 `tx.QueryRow(...authority_state...) FOR UPDATE`（PG）或事务写锁（SQLite），再调 fn（`role_policy.go:240-292`）。`patchRolePermissionsTx` 内 `roleTx(item.RoleID, true)` 取 FOR UPDATE（`role_policy.go:227-237`）。fn 内不再加用户锁，符合"fn must not lock further users, and policyWriteBatch is not reentrant"契约（`role_policy.go:240-243`）。`TestAssignUserRoleConcurrentExpectedVersionAllowsOne` 验证同 actor 不同角色的并发只允许一方成功（`role_policy_test.go:65-99`）。 | +| 政策锁后补锁用户 | 不存在 | fn 中所有用户相关写都基于 fn 入参 `targets`（已在 policyWriteBatch 入参时按 owner 序加锁），不再二次锁；`AccountDirectoryRevision` 等读面用独立只读 tx（`role_policy.go:1420-1476`）。 | + +### 重点 2 — patch 模式预览与批量提交语义一致性 + +| 子项 | 结论 | 依据 | +|---|---|---| +| 同一源 merge 函数（preview / commit） | PASS | `previewRolePermissionsTx` 在 `item.Mode=="patch"` 时调用 `mergeRolePermissionPatch(state.roles[role.RoleID], current, next)`（`role_policy.go:718-723`）；`patchRolePermissionsTx` 调用 `mergeRolePermissionPatch(previous.roles[role.RoleID], current, patch)`（`role_policy.go:1250`）；两路径共用同一函数（`role_policy.go:1168-1207`），预览看到的合并结果与提交落库结果一致。 | +| replace 路径回归风险 | PASS | Frontend `previewRoleBatch` 固定传 `mode: 'patch'`（`RolePermissionsPanel.tsx:96`），HTTP 预览请求体含 `mode:"patch"`（`api.ts:143-145`）；Frontend 不再以 `replace` 模式走批量事务。replace 模式仍由 `replaceRolePermissions` 单角色端点支持（`roles.go:103-117`、`role_policy.go:470-548`），未受 patch 改动回归。`TestPreviewRolePermissionsReplaceModeKeepsReplaceSemantics` 与 `TestRolePermissionPreviewPatchModeOverHTTP` 显式锁定两条路径语义。 | +| 旧项目 settings 关闭迁移文案 | PASS | `projectCreateDisabledPreviewNote` / `projectCreateDisabledNote` 在 disable patch 时给出中文说明（`role_policy.go:611-619`、`1825-1829`），`mergeRolePermissionPatch` 额外把 `current` 中存在但 patch 未列出的联动项加入 `plan.kept`（`role_policy.go:1192-1200`），保证 disable 后的 `KeptLinkedActions` 与 receipt 一致。`TestPreviewRolePermissionsReplaceModeKeepsReplaceSemantics`、`TestCapabilityAssociationDisablingProjectCreateKeepsLinkedRows` 验证。 | + +### 重点 3 — 前端多角色草稿隔离 / 统一保存 / 409 rebase / 跨页选择 / 复制并分配 + +| 子项 | 结论 | 依据 | +|---|---|---| +| 按 roleId 草稿隔离 | PASS | `RoleDraftMap = Record`（`RolePermissionsPanel.tsx:25-26`）；`editRoleDraft` 仅写 `drafts[role.roleId]`，跨角色互不污染（`RolePermissionsPanel.tsx:74-78`）；`roleCellValue`、`rolePatchEntries` 按 role 独立取基线（`rolePermissions.ts:74-78`、`97-103`）；`dirtyRoles` 仅列出 dirty role。`test/role-permissions.test.ts:79-90` 与 `test/role-permissions-ui.test.ts:44-58` 验证。 | +| 统一保存 | PASS | `previewRoleBatch` + `commitRoleBatch` 是唯一写入口（`RolePermissionsPanel.tsx:95-106`）；`confirmSave` 先以 `entries = patchEntries()` 调用，再 `setDrafts` 删除已保存角色（`RolePermissionsPanel.tsx:545-555`）。`TestRolePermissionUI.saves every dirty role through one authoritative preview and one batch commit` 验证两角色 dirty 时只发一次预览与一次提交。 | +| 409 rebase（按角色独立重放本地变化） | PASS | `rebaseRoleDrafts` 并行 `getRole(roleId)`，逐角色调用 `rebaseRoleDraft`（`RolePermissionsPanel.tsx:111-121`）；`rebaseRoleDraft` 复制 `serverValues`，仅在本地相对 `baseline` 变化时回放（`rolePermissions.ts:60-71`）。`test/role-permissions.test.ts:44-58` 与 `test/role-permissions-ui.test.ts:87-103` 验证。`rebaseRoleDrafts` 在 409 路径中被调用并设 `setPreview(null)`（`RolePermissionsPanel.tsx:558-563`）。 | +| 跨页选择模型（筛选不扩大集合） | PASS | `MemberSelection.ids` 仅来自逐个勾选或 `selectPage`，搜索/状态/翻页均不动 `selection.ids`（`roleMembers.ts:5-30`）。`selectPage(checked=true)` 只追加本页 id，不覆盖；`selectPage(checked=false)` 只移除本页 id，不动其他页。`test/role-members.test.ts:50-62` 显式断言"搜索变化 / 翻页不扩大集合"。 | +| 复制并分配（显式命名 + 权限来源） | PASS | `copyAssignRequest` 强制 `name.trim() != ''`、拒绝空 users 列表（`roleMembers.ts:107-115`）；overrides 仅含相对 source 的差异项（`roleMembers.ts:118-130`）；后端 `CreateRoleCopyAndAssign` 同步校验（`role_policy.go:1335-1398`），受保护动作不进副本（`role_policy.go:1395-1404`），新角色与源角色完全独立维护（`TestCopyAssignCreatesRoleAndBindsAtomically`）。权限来源（roleId/roleVersion）由 `source` 字段强制随请求传递且 `expectedRoleVersion` 不匹配返回 `ErrConflict`。 | + +### 重点 4 — 无障碍与设计守护（design-guard 棘轮） + +| 子项 | 结论 | 依据 | +|---|---|---| +| design-guard.test.ts 7 项 | PASS | `npx vitest run test/design-guard.test.ts` → `Test Files 1 passed (1), Tests 7 passed (7), Duration 127ms`。覆盖：DESIGN.md 宪法在位、动效唯一所有者、styles.css 裸色值基线、分层组件 CSS 裸色值、组件 TSX 裸色值、组件 TSX fontSize 数值字面量、组件 TSX 无 `
` 折叠。 | +| 新样式是否会破坏棘轮 | PASS | 本次新增样式集中在 `styles.css`（`.rp-` 与 `.au-` 前缀类，见 `styles.css:519-580` 与 `597-601`）。所用色全部来自 `var(--*)` 令牌，字体使用 `var(--fs-ui-*)`，唯一裸色字面量 `#0e1116 / #101826 / #0b1f38 / #dfe4f2`（行 448）属 HEX_BASELINE_STYLES 已登记的登录品牌面板；无 `@keyframes`，新增 transition 仅出现在 styles.css（设计守护本文件豁免）；新 TSX 文件（`RolePermissionsPanel.tsx`、`AccountUsersPanel.tsx`）无裸色字面量、无 fontSize 数值字面量、无 `
` 折叠——以 `@ui` Collapsible 替代（`RolePermissionsPanel.tsx` 全文件 `grep " !item.protected)`（`RolePermissionsPanel.tsx:521`）从矩阵排除这些动作。 | + +--- + +## 非阻塞建议（不阻塞 PR 合入；可作为后续增量） + +1. **跨角色 row-lock 顺序与 deadlock**：`PatchRolePermissionsBatch` / `AssignUserRolesBatch` / `CreateRoleCopyAndAssign` 内对角色行 `FOR UPDATE` 的顺序由请求 `items` 切片决定，未按 roleID 排序。`policyWriteBatch` 仅对 actor + target users 按 owner 排序；跨 actor + 跨事务的不同 `items` 顺序理论上可能触发 PG 死锁检测（事务回滚，非数据损坏）。建议：批量入口按 roleID 升序排序 `items` 后再入 fn，或由 `policyWriteBatch` 在 fn 之前排序 items。非阻塞，仅在极高并发 + 不同顺序组合下产生瞬时回滚。 + +2. **`searchAdminUsers` 返回完整 profile**：批量分配 picker 在选择用户时不需要 `phone / email / staffNo / department / notes / isAdmin / passwordSet / initializationState`；可考虑按 view 端点（`system.users.view`）与 assign 端点（`system.roles.assign`）返回裁剪字段，或新增 `fields` 显式控制响应字段（现有 `fields` 仅控制过滤字段）。非阻塞，属既有端点行为。 + +3. **`roleDraftLeaveGuard.current = null` 在 unmount 时由 effect cleanup 复位**：`RolePermissionsPanel.tsx:431` 的 effect cleanup 会在依赖变化（`dirtyKey` 变化）时立即把 `current` 置 null；这与"新守卫被设置"的顺序是先 set 后清，存在极短的 `current === null` 间隙（毫秒级）。在 AdminZone 返回按钮的 click 期间几乎不会被命中；如需更稳，可改用 `useRef` 直接保存守卫函数 + `useCallback`，避免 effect 驱动的 reset。非阻塞。 + +4. **`canManagePermissions` 取 view 失败时不显示空态**：当 `permissionCatalog()` 拒绝时，`load` 会清空 `catalogFailed=true` 并在选中角色之后追加错误（`RolePermissionsPanel.tsx:501-507`）。与原 DR10 备注一致（保留部分失败可见性），实现正确；建议把 `editable` 同步置 `false` 并在 `focus()` 内显式提示当前矩阵不可编辑，以避免在慢网络下"已切换单角色视图 → 单元格短暂可编辑 → catalog 失败回填"造成闪烁。属 UX 微调，非阻塞。 + +5. **`PATCH /api/me/profile` 与权限 patch 两条路径的 `adoptRoleSnapshot` 语义**：当 `PATCH /admin/roles/{id}`（资料 / 启停）成功后 `setDrafts(prev => adoptRoleSnapshot(prev, selected.roleVersion, updated))`（`RolePermissionsPanel.tsx:488`）继承新 roleVersion；但权限草稿在资料/启停 patch 之后仍属同一角色 → 同一基线；当前实现对 `adoptRoleSnapshot` 的判定是"当前基线 roleVersion === 期望 roleVersion 时采纳新 row"，逻辑正确，仅在资料 patch 与权限 patch 并发时存在 `expectedVersion != 实际 version` 的窗口（被下一次 save 的 409 路径正确处理）。建议在 `adoptRoleSnapshot` 注释中明示"用于 PATCH 后的非权限写成功继承"。非阻塞。 + +--- + +## 不属于本任务范围（按任务卡 NOT_RUN 处置，未列入阻塞/建议） + +- C13 成员共享读取（依赖 L1 U7） +- C14 完整键盘 / 读屏人工走查 +- C16 日常实例采用（未授权部署） + +--- + +## 引用文件位置索引 + +- Backend：`.task/feature-iam-role-permission-linked-capabilities-perm-ux.md` → 组件 commit `modules/backend @ 4130ad78` + - 关键文件：`modules/backend/internal/store/role_policy.go`、`modules/backend/internal/server/roles.go`、`modules/backend/internal/server/account_search.go`、`modules/backend/internal/authz/catalog/catalog.go`、`modules/backend/internal/authz/service.go`、`modules/backend/pkg/extension/role_policy.go` + - 单测：`modules/backend/internal/store/role_policy_test.go`、`modules/backend/internal/server/role_authorization_test.go`、`modules/backend/internal/authz/catalog/catalog_test.go` +- Frontend：组件 commit `modules/frontend @ 46a63f02` + - 关键文件：`modules/frontend/src/components/RolePermissionsPanel.tsx`、`modules/frontend/src/components/AccountUsersPanel.tsx`、`modules/frontend/src/components/adminRdmSections.tsx`、`modules/frontend/src/rolePermissions.ts`、`modules/frontend/src/roleMembers.ts`、`modules/frontend/src/permissionExplanations.ts`、`modules/frontend/src/api.ts`、`modules/frontend/src/styles.css` + - 单测：`modules/frontend/test/design-guard.test.ts`、`modules/frontend/test/role-permissions.test.ts`、`modules/frontend/test/role-permissions-ui.test.ts`、`modules/frontend/test/role-members.test.ts`、`modules/frontend/test/permission-explanations.test.ts`、`modules/frontend/test/account-users-panel.test.ts` + - 活体：`modules/frontend/e2e/iam-perm-ux-live.playwright.ts` +- Dev 父仓：当前 `feature/iam-role-permission-linked-capabilities-perm-ux @ 234ce30`（基线 90317d02；组合 gitlink `modules/backend @ 4130ad7` + `modules/frontend @ 46a63f0`）