Repository navigation
Sharing-service hardening: rule-name length bound, legacy plan backfill, reconcile branch coverage - #16
Merged
Conversation
… reconcile coverage Six items from the PR #4 review, refined as work item #5: 1. Rule-name length bound. `sys_sharing_rule.name` is capped at 100 while the worst-case derived name was 116 (`kpi_p{plan}_result_leader_{unit}_{leader}` with three 32-char compliant ids). Every id fragment is now bounded at 26 via `RULE_SLUG_MAX`, so the longest shape is exactly 99. Over-long but otherwise compliant ids are truncated to 17 chars plus an 8-char stable hash; derivation stays idempotent across publishes. The existing fold for UUID-shaped ids (head 12 + stable hash) is deliberately untouched: changing it would rename every rule written before the upgrade, and the old names would sit neither in the desired set nor under the new plan prefix, so reconcile could never reclaim them again — the exact unreclaimable authorisation this module exists to prevent. A test pins that fold. 2. Legacy row backfill. New `src/data/backfill-plan-links.ts`, hooked on `kernel:bootstrapped` next to `align-demo-units`. Rows of `kpi_check_task` / `kpi_adjustment` created before the redundant `plan` column was written have an empty `plan` and therefore match no plan-scoped rule criteria. The backfill fills `plan` from the row's own entry sheet — only that one field, only rows where it is empty, resolved via the system context. Rows a hook refuses are counted and skipped, never written around. Idempotent: nothing to fill means zero writes and no log line. 3. Reconcile branch coverage. Unit tests for the legacy `kpi_share_*` deactivation branch in both directions (legacy rule present → deactivated; absent → nothing touched), plus other-plan and admin-customised rules staying out of range. 4. Log distinction. `skipped_inactive` splits into `skipped_inactive_admin` (an admin withdrew the grant in Setup) and `skipped_inactive_reconciled` (this app deactivated it during its own reconcile), told apart by a description prefix so no field is added to the platform object. 5. Null-organization refusal stays strict; the ruling and its date are recorded at the refusal site. 6. Re-assert fan-out is recorded as a known trade-off in a comment, not changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb
… where it is visible F1 (blocking). `planOfSheets` called `ctx.ql.find` with no try/catch while the whole chain hangs off `kernel:bootstrapped`, so one failed read — a 500-id `$in` hitting a driver parameter limit, say — was an unhandled rejection during boot that could interrupt the app and would skip the `kpi_adjustment` pass entirely. The lookup now catches, warns, and returns an empty map: the batch falls into `unresolved` and the next start retries it. `registerPlanLinkBackfill`'s `run` also wraps everything in a catch, the same shape `align-demo-units.ts` uses, so `run` cannot throw regardless of whether the inner layers keep behaving. Two tests cover it: a failing sheet lookup neither throws nor stops the second object, and the registered handler resolves even when every read fails. F3. A full batch that made no progress used to stop the loop silently, which reads exactly like "backfill finished" while rows may still be waiting. It now leaves a warn naming the object and the counts. Test included. F6. The summary logs at `warn` when it actually filled rows: dev's default log level does not show `info`, and "this start rewrote N historical rows" is the one line that must be visible. Nothing is logged when there was nothing to fill. Test asserts the level. F2 (comment only). The rule-name note said the over-long branch was the only derivation this change moves. Two segments move, not one: 27-32 char compliant ids (the only ones that overflowed) and 33+ char compliant ids, which switch from head+hash to truncate+hash without ever having overflowed. Neither shape occurs in this app — platform record ids are UUIDs and app-defined ids are short names. F4 (comment only). The write-refused path is documented as future-proofing rather than as something that happens today: every hook in this repo passes system context through. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work item #5 — the six items left by the PR #4 review, refined by the dispatcher on 2026-09-02. Baseline main @ bdc87ff, final commit
888e642.六条逐项
1. 规则名长度上界(
src/services/sharing-service.ts)sys_sharing_rule.name上限 100,而派生名最坏拼到 117 —— 入口只有一个:ruleSlug对「合规且不超过 32 位」的 id 原样保留,三段 32 位 id 拼成kpi_p{方案}_result_leader_{单元}_{领导}= 117。新增RULE_NAME_MAX = 100与RULE_SLUG_MAX = 26:片段各 26 时最长形态正好 99。合规但超长的 id 折成「截断 17 位 + 8 位稳定哈希」,长度恒为 26,幂等。UUID 形态 id 的既有折法(头部 12 位 + 稳定哈希)一个字都没动 —— 改了它,升级前已写入的规则会整批改名而无法被对账回收。有一条单测把这条折法钉死;评审员用 3,500 个 id 做老/新差分,零漂移。
2. 历史行回填(新增
src/data/backfill-plan-links.ts)升级前建的
kpi_check_task/kpi_adjustment行plan为空,落不进任何按方案的规则条件。新模块挂在kernel:bootstrapped(与align-demo-units同一挂点),在objectstack.config.ts的onEnable注册。三重收窄:只写plan一个字段;只动plan为空的行;方案取自该行自己的填报单,推不出就计入unresolved跳过。写被拒的行计入failed并记日志,不绕过 hook 直写。幂等:补齐之后再启动零写入、零日志。3. 对账分支测试
reconcilePlanSharing停用旧式kpi_share_*规则的分支补了两个方向的单测,外加「别的方案的规则」与「管理员改过的规则」都不在射程内。4. 日志区分
skipped_inactive拆成skipped_inactive_admin与skipped_inactive_reconciled,以规则描述前缀区分(RECONCILED_DESCRIPTION_MARK),不给平台对象加字段;两种情形都仍然绝不改写active。5. 空组织策略:维持拒绝(裁定 2026-09-02,注释已补)
6. 重申扇出:只记录,不动(
provisionPlanSharing文档注释)Review round 1 —— 追加提交
888e642planOfSheets()的填报单读取包 try/catch,失败时 warn 并返回空 Map(该批落unresolved,下次启动再补);run外再包一层 catch(与align-demo-units.ts同形态),回填在启动期不可能抛。两条单测:读失败不抛、不阻断另一个对象;注册入口在每次读都失败时仍 resolves。$nin,它会撞与 F1 同类的参数上限)。filled > 0时回填摘要走warn级(dev 默认级别可见),零回填仍不打印。ruleSlug与回填注释订正。验证(
888e642)pnpm verify绿:validate 通过、typecheck 无输出、vitest 90 passed (6 files)。rm -rf dist+ 删库后重启 dev,pnpm e2e:73/73,T39rules=29 expected=29。WARN [kpi] 历史行方案回填完成 {"filled":1,"unresolved":0,"failed":0,…};该行经 REST 读回plan已填;再次启动回填日志 0 行。不在本次范围
docs/00~docs/03、README、CLAUDE.md、.github/均未触碰;docs/04只在 #18 的备注里补了一句「历史行回填」。Review gate: round 1 NOT MERGEABLE (1 should-fix), round 2 MERGEABLE at 888e642; dispatcher collection check passed at both rounds.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb