docs(02): add code review fix report
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,79 @@
|
|||||||
|
---
|
||||||
|
phase: 02-api-parity-harness-bootstrap
|
||||||
|
fixed_at: 2026-09-17T12:55:00Z
|
||||||
|
review_path: .planning/phases/02-api-parity-harness-bootstrap/02-REVIEW.md
|
||||||
|
iteration: 1
|
||||||
|
findings_in_scope: 8
|
||||||
|
fixed: 8
|
||||||
|
skipped: 0
|
||||||
|
status: all_fixed
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 02: Code Review Fix Report
|
||||||
|
|
||||||
|
**Fixed at:** 2026-09-17T12:55:00Z
|
||||||
|
**Source review:** `.planning/phases/02-api-parity-harness-bootstrap/02-REVIEW.md`
|
||||||
|
**Iteration:** 1
|
||||||
|
|
||||||
|
**Summary:**
|
||||||
|
- Findings in scope: 8 (CR-01, WR-01 … WR-07)
|
||||||
|
- Fixed: 8
|
||||||
|
- Skipped: 0
|
||||||
|
- Info (IN-01 … IN-04): not in scope
|
||||||
|
|
||||||
|
## Fixed Issues
|
||||||
|
|
||||||
|
### CR-01: Short captured IDs rewrite pagination and IPv4, hiding PHP/Go diffs
|
||||||
|
|
||||||
|
**Files modified:** `tide/variables.go`, `tide/capture_test.go`; `../fonoteka.go/parity/fixtures/**` (38 YAML files)
|
||||||
|
**Commit:** `5994e67` (framework), `d6251e0` (fonoteka fixtures)
|
||||||
|
**Status:** fixed: requires human verification
|
||||||
|
**Applied fix:** Short numeric capture values (`len < 8`) are no longer substituted into request/response bodies. Path, query, and headers still isolate `/albums/1`. `.` is an identifier byte so IPv4 last octets stay literal. Added `TestScrubShortIDsLeavePaginationAndIPv4Literal`. Restored committed pagination keys (`current_page`, `last_page`, `total`, `album_count`, counts) and `127.0.0.{{id:*}}` to literal `1` / `127.0.0.1`. JSON `id` / `*_id` placeholders were left in place; `normalizeJSON` still masks those at compare time.
|
||||||
|
|
||||||
|
### WR-01: Fixture save is neither exclusive nor all-or-nothing
|
||||||
|
|
||||||
|
**Files modified:** `tide/fixture.go`, `tide/proxy.go`, `tide/proxy_test.go`, `tide/manifest.go`, `cmd/summer/parity.go`
|
||||||
|
**Commit:** `7e70afe`
|
||||||
|
**Applied fix:** Sessions stay in memory until `Flush`. `SaveFlowExclusive` claims the destination with `O_EXCL`. Failed sessions always `os.Remove` the session path. `parity:proxy --update` overwrites. Manifest route recording uses exclusive create unless `--update`.
|
||||||
|
|
||||||
|
### WR-02: Redirect `Location` is recorded but never compared
|
||||||
|
|
||||||
|
**Files modified:** `tide/diff.go`, `tide/headers_test.go`
|
||||||
|
**Commit:** `5cb2def`
|
||||||
|
**Applied fix:** Added `Location` to `extraCompareHeaders`. `Set-Cookie` stays off the compare list.
|
||||||
|
|
||||||
|
### WR-03: JSON decoder stops after the first value
|
||||||
|
|
||||||
|
**Files modified:** `tide/diff.go`, `tide/diff_test.go`
|
||||||
|
**Commit:** `d3d202e`
|
||||||
|
**Applied fix:** `decodeJSON` errors when `Decoder.More()` is true after the first value.
|
||||||
|
|
||||||
|
### WR-04: Secret scanners miss passwords and live mismatch text can print tokens
|
||||||
|
|
||||||
|
**Files modified:** `tide/variables.go`, `tide/flow.go`, `tide/manifest.go`, `tide/capture_test.go`; `../fonoteka.go/parity/check_corpus.go`, `../fonoteka.go/parity/check_corpus_test.go`
|
||||||
|
**Commit:** `0ac9dc4` (framework), `1336d53` (fonoteka)
|
||||||
|
**Applied fix:** After placeholder strip, leftover `"password"` / `password=` fail unless the value is the documented allow-list `parity-alice-pass`. Opaque `"access_token"` leftovers fail. `MismatchError` and coverage diff lines run through `redactSecrets` (JWT, `inv_`, `client_secret`, `auth_token`).
|
||||||
|
|
||||||
|
### WR-05: `php_parity.sh` isolation is a basename substring
|
||||||
|
|
||||||
|
**Files modified:** `../fonoteka.go/parity/php_parity.sh`
|
||||||
|
**Commit:** `d0f1b61`
|
||||||
|
**Applied fix:** Basename must still contain `parity`, and the resolved path must live under `$PARITY_ROOT` (`/tmp/summercms-parity` by default). Any path inside `$PHP_ROOT` is refused.
|
||||||
|
|
||||||
|
### WR-06: Parity Postgres container is leaked on `os.Exit`
|
||||||
|
|
||||||
|
**Files modified:** `../fonoteka.go/parity/synthetic_test.go`
|
||||||
|
**Commit:** `fb64174`
|
||||||
|
**Applied fix:** `stopParityPostgres()` is called explicitly before every `os.Exit` in `TestMain`. `defer` is no longer used for container cleanup.
|
||||||
|
|
||||||
|
### WR-07: Fixture/vars path checks do not evaluate symlinks
|
||||||
|
|
||||||
|
**Files modified:** `tide/variables.go`, `tide/flow.go`, `tide/manifest.go`, `tide/proxy_test.go`
|
||||||
|
**Commit:** `f7b81b9`
|
||||||
|
**Applied fix:** `resolvePath` uses `EvalSymlinks` on the path or its parent before prefix checks. Sidecar reads must stay inside the fixture root after symlink resolution. A vars symlink into `fixtures/` is rejected.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
_Fixed: 2026-09-17T12:55:00Z_
|
||||||
|
_Fixer: the agent (gsd-code-fixer)_
|
||||||
|
_Iteration: 1_
|
||||||
Reference in New Issue
Block a user