---
phase: 08-screenshot-profiles
fixed_at: 2026-04-13T00:00:00Z
review_path: .planning/phases/08-screenshot-profiles/08-REVIEW.md
iteration: 1
findings_in_scope: 3
fixed: 3
skipped: 0
status: all_fixed
---

# Phase 8: Code Review Fix Report

**Fixed at:** 2026-04-13
**Source review:** .planning/phases/08-screenshot-profiles/08-REVIEW.md
**Iteration:** 1

**Summary:**
- Findings in scope: 3
- Fixed: 3
- Skipped: 0

## Fixed Issues

### WR-01: Nullish coalescing prevents overriding profile fields with zero

**Files modified:** `src/profiles/profile-resolver.ts`
**Commit:** bc21534
**Applied fix:** Added documentation comment to `resolveScreenshotProfile` explaining the nullish coalescing override semantics -- that `undefined` means "not provided" and there is no way to explicitly clear a profile field. Suggests using `null` as "clear this field" if needed in the future.

### WR-02: Fragile default-value sentinel in profile target resolution

**Files modified:** `src/server.ts`
**Commit:** 4579f52
**Applied fix:** Changed `target` schema from `.default("desktop")` to `.optional()`, and moved the "desktop" default to the runtime logic (`args.target ?? "desktop"`). The profile resolver now receives `args.target` directly (without sentinel stripping), so an explicit `target: "desktop"` is correctly distinguished from "not provided" and properly overrides a profile's non-desktop target. Status: fixed: requires human verification (logic change).

### WR-03: Concurrent profile writes can lose data

**Files modified:** `src/profiles/profile-manager.ts`
**Commit:** cec67ac
**Applied fix:** Added a `persistQueue` promise chain to the `ProfileManager` class. The `persist()` method now chains writes through `this.persistQueue.then(...)` so concurrent save/delete calls are serialized, preventing the TOCTOU race where one write could overwrite another.

---

_Fixed: 2026-04-13_
_Fixer: Claude (gsd-code-fixer)_
_Iteration: 1_