[Review] @ts-nocheck opt-out on src/lib/intimate-state-engine.ts and src/vault/VaultStore.ts — type-safety hole in privacy subsystem #176

Closed
opened 2026-09-14 14:07:04 +00:00 by Mark · 0 comments
Owner

Finding

Two large files added in #168/#169 ship with // @ts-nocheck at the top:

  • src/lib/intimate-state-engine.ts:1-2 (540 lines)
  • src/vault/VaultStore.ts:1-2 (591 lines)
/* eslint-disable @typescript-eslint/ban-ts-comment */
// @ts-nocheck

The intimate-state engine is the public contract for the new UI. VaultStore is the privacy boundary — the literal code that gates sensitive fields behind assertUnlocked(). Both are exactly the places type checking matters most.

What this enables

  • BehaviorPatternInsight.category is a closed union in src/lib/types.ts:902-912. PersonDetail.tsx filters visible insights by SENSITIVE_CATEGORIES.has(i.category) where the set is ["mood","conflict","cycle","precursor","vulnerability"]. If a new sensitive category is added to the union (e.g. "intimacy"), the PersonDetail filter set must be hand-updated in lockstep. With @ts-nocheck'd producers, the new category is silently accepted; the two lists can drift.
  • confidence: "high" | "medium" | "low" — narrowings like riskScore >= 4 ? "high" : "medium" are arithmetic-only, not type-narrowed. A typo ("hihg") compiles.
  • The as VaultMoodEntry / as Interaction casts in VaultStore suggest the actual problem is a real type-definition gap (e.g. wanting to assign undefined to a field whose zod schema requires a string). Disabling types papers over a legitimate schema/runtime mismatch rather than fixing it.

Suggested fix

  1. Remove the pragma from both files.
  2. Run tsc -b --noEmit, fix the underlying errors. Expect:
    • The mood: undefined assignment in migrateSensitiveFieldsToVault (VaultStore.ts:529) conflicts with the zod mood: z.string() requirement on Interaction. The right fix is to mark these fields as optional in the Interaction schema (mood?: string) so the migration can clear them.
    • The as Person casts in addPerson / moveFromMain need either a typed factory or explicit field-by-field assignment.
  3. Verify tsc clean and vitest run still passes (1214 tests).

Severity

Medium-high. Not a runtime crash today, but a maintainability and contract-drift hazard in the privacy subsystem. Every future contributor touching these files can introduce type errors that compile but break the contract with PersonDetail.

Discovered by

Manual code review pass over #168/#169, 2026-09-14.

## Finding Two large files added in #168/#169 ship with `// @ts-nocheck` at the top: - `src/lib/intimate-state-engine.ts:1-2` (540 lines) - `src/vault/VaultStore.ts:1-2` (591 lines) ```ts /* eslint-disable @typescript-eslint/ban-ts-comment */ // @ts-nocheck ``` The intimate-state engine is the public contract for the new UI. VaultStore is the privacy boundary — the literal code that gates sensitive fields behind `assertUnlocked()`. Both are exactly the places type checking matters most. ## What this enables - `BehaviorPatternInsight.category` is a closed union in `src/lib/types.ts:902-912`. PersonDetail.tsx filters visible insights by `SENSITIVE_CATEGORIES.has(i.category)` where the set is `["mood","conflict","cycle","precursor","vulnerability"]`. If a new sensitive category is added to the union (e.g. `"intimacy"`), the PersonDetail filter set must be hand-updated in lockstep. With @ts-nocheck'd producers, the new category is silently accepted; the two lists can drift. - `confidence: "high" | "medium" | "low"` — narrowings like `riskScore >= 4 ? "high" : "medium"` are arithmetic-only, not type-narrowed. A typo (`"hihg"`) compiles. - The `as VaultMoodEntry` / `as Interaction` casts in VaultStore suggest the actual problem is a real type-definition gap (e.g. wanting to assign `undefined` to a field whose zod schema requires a string). Disabling types papers over a legitimate schema/runtime mismatch rather than fixing it. ## Suggested fix 1. Remove the pragma from both files. 2. Run `tsc -b --noEmit`, fix the underlying errors. Expect: - The `mood: undefined` assignment in `migrateSensitiveFieldsToVault` (VaultStore.ts:529) conflicts with the zod `mood: z.string()` requirement on `Interaction`. The right fix is to mark these fields as optional in the Interaction schema (`mood?: string`) so the migration can clear them. - The `as Person` casts in `addPerson` / `moveFromMain` need either a typed factory or explicit field-by-field assignment. 3. Verify tsc clean and `vitest run` still passes (1214 tests). ## Severity Medium-high. Not a runtime crash today, but a maintainability and contract-drift hazard in the privacy subsystem. Every future contributor touching these files can introduce type errors that compile but break the contract with PersonDetail. ## Discovered by Manual code review pass over #168/#169, 2026-09-14.
Mark 2026-09-14 16:25:50 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Mark/kimi_agent_kin_app_sync#176
No description provided.