Refinement Audit — Cycle 3

Summary

Cycle 2 removed the most obvious desktop edge failures, and the remaining defects are now concentrated in contract enforcement and publish-time repository safety. The highest-risk issue is a git publish path that can commit unrelated pre-staged work, with two additional correctness gaps in settings validation and storm-folder root handling.

Critical Findings

C1: Publish on the target branch can commit unrelated pre-staged files

  • File: src/main/services/git/PublishService.ts:319
  • Issue: The publish flow only auto-stashes a dirty worktree when it first has to switch branches. If the user is already on the target branch and the index contains unrelated staged files, git add stages the requested publish files on top of that existing index state and the later commit includes both sets of changes.
  • Evidence: startDirty is computed at src/main/services/git/PublishService.ts:317, but the stash branch is gated by if (branchInfo.current !== targetBranch) at src/main/services/git/PublishService.ts:319. The later commit path never clears or validates pre-existing staged entries before git.commit(...).
  • Fix: Before staging, inspect the current staged paths and reject the publish when the index already contains paths outside the requested publish set, with a dedicated user-facing error and regression coverage for the same-branch case.

Important Findings

I1: settings:update silently accepts unknown keys and reports success

  • File: src/main/ipc/handlers/settingsHandlers.ts:303
  • Issue: The update path validates the merged config with unknownKeyPolicy: 'strip', so typoed or version-skewed fields are discarded instead of rejected. Callers can receive a successful response even though the requested field was never applied.
  • Evidence: deepMergeAppConfigWithPatch(...) preserves arbitrary keys, then parseAppConfig(merged as unknown, { unknownKeyPolicy: 'strip' }) at src/main/ipc/handlers/settingsHandlers.ts:303 removes them and the handler proceeds to save and return success.
  • Fix: Validate settings updates with unknownKeyPolicy: 'reject' (or an equivalent explicit patch-key allowlist) so unsupported fields fail with SETTINGS_VALIDATION_ERROR, and add a regression test for an unknown-key payload.

I2: Storm folder creation recreates a missing tropical root instead of failing fast

  • File: src/main/services/storms/StormFolderService.ts:472
  • Issue: createFolder never verifies that the configured tropical updates root already exists as a directory. On a deleted or mistyped base path, the recursive mkdir call creates a brand-new root tree and reports success, masking the configuration problem and potentially writing folders into the wrong location.
  • Evidence: handleCreateFolder resolves root at src/main/services/storms/StormFolderService.ts:441, checks only dest existence at src/main/services/storms/StormFolderService.ts:455-468, then calls mkdir(dest, { recursive: true }) at src/main/services/storms/StormFolderService.ts:472 without any prior stat(root) or stat(yearDir) validation.
  • Fix: Verify that the configured tropical root exists and is a directory before any create/update/sync operation, and surface a structured filesystem error instead of recursively creating the missing base tree.

Minor Findings

None.

Design Improvements

None.

Regressions

None detected.

TOTAL_FINDINGS: 3