#184 - [Review] Vault-gate scrub list duplicated in src/lib/store.ts:1001 and src/pages/PersonDetail.tsx:2967 — should be a single helper - Mark/kimi_agent_kin_app_sync - cyguin git: Adlerian Hierarchical Self-Actualization Syllogism

Mark commented

2026-09-14 14:07:30 +00:00

Owner

Finding

Two sites maintain the exact same list of sensitive fields to scrub when vaultLocked === true:

src/lib/store.ts:1001-1014:

const safeInteractions = state.vaultLocked
  ? state.interactions.map((i) => ({
      ...i,
      mood: undefined,
      intimacyLevel: undefined,
      isConflict: undefined,
      repairAttempt: undefined,
      conflictResolved: undefined,
      qualityRating: undefined,
    }))
  : state.interactions;

src/pages/PersonDetail.tsx:2967-2974 (inside MetricsTab):

const safeInteractions = useMemo(() => {
  if (!vaultLocked) return interactions;
  return interactions.map((i) => ({
    ...i,
    mood: undefined,
    intimacyLevel: undefined,
    isConflict: undefined,
    repairAttempt: undefined,
    conflictResolved: undefined,
    qualityRating: undefined,
  }));
}, [interactions, vaultLocked]);

Adding a new sensitive field to Interaction (e.g. repairDate, conflictResolutionNotes) requires updating both lists in lockstep. There's no shared scrubSensitiveFields(interactions) helper.

Suggested fix

Export a single helper, e.g. in src/vault/vault-metrics-engine.ts or a new src/vault/scrub.ts:

export function scrubSensitiveInteractions(
  interactions: Interaction[],
): Interaction[] {
  return interactions.map((i) => ({
    ...i,
    mood: undefined,
    intimacyLevel: undefined,
    isConflict: undefined,
    repairAttempt: undefined,
    conflictResolved: undefined,
    qualityRating: undefined,
  }));
}

export const SENSITIVE_INTERACTION_FIELDS = [
  "mood", "intimacyLevel", "isConflict", "repairAttempt",
  "conflictResolved", "qualityRating",
] as const;

Call from both sites. Add a unit test that mutates a new field on a sample Interaction and asserts the helper clears it.

Severity

Medium. Drift hazard. Each new sensitive field is a double-edit; missing one site = a privacy leak.

Discovered by

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

## Finding Two sites maintain the exact same list of sensitive fields to scrub when `vaultLocked === true`: `src/lib/store.ts:1001-1014`: ```ts const safeInteractions = state.vaultLocked ? state.interactions.map((i) => ({ ...i, mood: undefined, intimacyLevel: undefined, isConflict: undefined, repairAttempt: undefined, conflictResolved: undefined, qualityRating: undefined, })) : state.interactions; ``` `src/pages/PersonDetail.tsx:2967-2974` (inside `MetricsTab`): ```ts const safeInteractions = useMemo(() => { if (!vaultLocked) return interactions; return interactions.map((i) => ({ ...i, mood: undefined, intimacyLevel: undefined, isConflict: undefined, repairAttempt: undefined, conflictResolved: undefined, qualityRating: undefined, })); }, [interactions, vaultLocked]); ``` Adding a new sensitive field to `Interaction` (e.g. `repairDate`, `conflictResolutionNotes`) requires updating both lists in lockstep. There's no shared `scrubSensitiveFields(interactions)` helper. ## Suggested fix Export a single helper, e.g. in `src/vault/vault-metrics-engine.ts` or a new `src/vault/scrub.ts`: ```ts export function scrubSensitiveInteractions( interactions: Interaction[], ): Interaction[] { return interactions.map((i) => ({ ...i, mood: undefined, intimacyLevel: undefined, isConflict: undefined, repairAttempt: undefined, conflictResolved: undefined, qualityRating: undefined, })); } export const SENSITIVE_INTERACTION_FIELDS = [ "mood", "intimacyLevel", "isConflict", "repairAttempt", "conflictResolved", "qualityRating", ] as const; ``` Call from both sites. Add a unit test that mutates a new field on a sample Interaction and asserts the helper clears it. ## Severity Medium. Drift hazard. Each new sensitive field is a double-edit; missing one site = a privacy leak. ## Discovered by Manual code review pass over #169, 2026-09-14.

Mark added the

in-progress

label

2026-09-14 14:07:30 +00:00

Mark commented

2026-09-14 14:37:39 +00:00

Author

Owner

Closing as already-fixed.

Location: 0854652 (on main)

Change: fix(#184): extract vault-gate scrub helper, dedupe #169 scrub lists

The chat-agent code review read against a stale HEAD and reported this issue as open. The fix was already in place (committed for #184, in the kilo-gsd worktree for #185) when the review started.

Stash + worktree state at session start:

HEAD was at 0854652 (fix #184), not 4fe2244 as the session began assuming.
stash@{0} contained leftover WIP for #183 (now redundant; #183 is committed).
Working tree held #185 fix (code + regression test) uncommitted.

No work to do. Re-classify if either fix needs to be re-merged to a different branch.

Closing as already-fixed. Location: 0854652 (on main) Change: fix(#184): extract vault-gate scrub helper, dedupe #169 scrub lists The chat-agent code review read against a stale HEAD and reported this issue as open. The fix was already in place (committed for #184, in the kilo-gsd worktree for #185) when the review started. Stash + worktree state at session start: - HEAD was at 0854652 (fix #184), not 4fe2244 as the session began assuming. - stash@{0} contained leftover WIP for #183 (now redundant; #183 is committed). - Working tree held #185 fix (code + regression test) uncommitted. No work to do. Re-classify if either fix needs to be re-merged to a different branch.

Mark closed this issue

2026-09-14 14:37:39 +00:00

Mark referenced this issue

2026-09-14 14:37:39 +00:00

[Review] detectExcitementCycles bag-of-words picks up common English words ('time', 'meeting', 'just') as 'excitement keywords' #185

Mark referenced this issue

2026-09-14 14:37:53 +00:00

[Review] computeVaultMoodTrend anchors window on newest entry, but computeMoodTrend (#171) anchors on Date.now() — opposite choices for the same signal #181

Mark removed the

in-progress

label

2026-09-14 14:38:04 +00:00