Refinement Audit — Cycle 3
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 addstages the requested publish files on top of that existing index state and the later commit includes both sets of changes. - Evidence:
startDirtyis computed atsrc/main/services/git/PublishService.ts:317, but the stash branch is gated byif (branchInfo.current !== targetBranch)atsrc/main/services/git/PublishService.ts:319. The later commit path never clears or validates pre-existing staged entries beforegit.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, thenparseAppConfig(merged as unknown, { unknownKeyPolicy: 'strip' })atsrc/main/ipc/handlers/settingsHandlers.ts:303removes 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 withSETTINGS_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:
createFoldernever verifies that the configured tropical updates root already exists as a directory. On a deleted or mistyped base path, the recursivemkdircall creates a brand-new root tree and reports success, masking the configuration problem and potentially writing folders into the wrong location. - Evidence:
handleCreateFolderresolvesrootatsrc/main/services/storms/StormFolderService.ts:441, checks onlydestexistence atsrc/main/services/storms/StormFolderService.ts:455-468, then callsmkdir(dest, { recursive: true })atsrc/main/services/storms/StormFolderService.ts:472without any priorstat(root)orstat(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