fix: address code review findings across backend and frontend #46

2026-05-21T20:03:36Z

lerko commented

2026-05-21 20:03:36 +00:00

Summary

ULID goroutine safety: Wrap MonotonicEntropy in LockedMonotonicReader + concurrent uniqueness test
Migration PRAGMA bug: Move PRAGMA foreign_keys = OFF/ON outside transaction where SQLite was ignoring it; remove dead currentSchema constant
LIKE wildcard injection: Escape %, _, \ in link resolution queries with ESCAPE '\' clause
Non-localhost warning: Print stderr warning when binding to non-localhost with no auth
writeJSON error logging: Log json.Encoder.Encode() failures instead of silently dropping them
DELETE semantics: Add ?permanent=true query param for explicit hard delete + HardDelete store method
Frontend resp.ok checks: Add checkedJSON() wrapper; all API methods now validate HTTP status before parsing
stopPropagation consistency: All action buttons in renderStreamPeek and renderCardPeek now use event.stopPropagation()
DRY section rendering: Extract renderCardSections() shared by renderInlineDetail and renderCardPeek
Absorb metadata preservation: Inherit source title/description when target has none
Backup collision: Millisecond-precision timestamps in auto-generated backup paths
spaHandler path safety: Add explicit path.Clean before extension check

Test plan

go test -race ./... passes (all packages)
go build ./... clean
Concurrent ULID test (100 goroutines) passes with race detector
Manual: verify non-localhost warning prints with --host 0.0.0.0
Manual: verify DELETE /api/entities/:id?permanent=true hard-deletes
Manual: verify frontend shows error on API failure (e.g. stop server mid-session)

## Summary - **ULID goroutine safety**: Wrap `MonotonicEntropy` in `LockedMonotonicReader` + concurrent uniqueness test - **Migration PRAGMA bug**: Move `PRAGMA foreign_keys = OFF/ON` outside transaction where SQLite was ignoring it; remove dead `currentSchema` constant - **LIKE wildcard injection**: Escape `%`, `_`, `\` in link resolution queries with `ESCAPE '\'` clause - **Non-localhost warning**: Print stderr warning when binding to non-localhost with no auth - **writeJSON error logging**: Log `json.Encoder.Encode()` failures instead of silently dropping them - **DELETE semantics**: Add `?permanent=true` query param for explicit hard delete + `HardDelete` store method - **Frontend resp.ok checks**: Add `checkedJSON()` wrapper; all API methods now validate HTTP status before parsing - **stopPropagation consistency**: All action buttons in renderStreamPeek and renderCardPeek now use `event.stopPropagation()` - **DRY section rendering**: Extract `renderCardSections()` shared by `renderInlineDetail` and `renderCardPeek` - **Absorb metadata preservation**: Inherit source title/description when target has none - **Backup collision**: Millisecond-precision timestamps in auto-generated backup paths - **spaHandler path safety**: Add explicit `path.Clean` before extension check ## Test plan - [x] `go test -race ./...` passes (all packages) - [x] `go build ./...` clean - [x] Concurrent ULID test (100 goroutines) passes with race detector - [ ] Manual: verify non-localhost warning prints with `--host 0.0.0.0` - [ ] Manual: verify `DELETE /api/entities/:id?permanent=true` hard-deletes - [ ] Manual: verify frontend shows error on API failure (e.g. stop server mid-session)

lerko added 1 commit 2026-05-21 20:03:36 +00:00

fix: address code review findings across backend and frontend

CI / test (pull_request) Successful in 2m13s

Details

e9ecc4c1f7

Fix goroutine-unsafe ULID entropy by wrapping in LockedMonotonicReader.
Move PRAGMA foreign_keys outside transaction in v3 migration where
SQLite was silently ignoring it. Escape LIKE wildcards in link
resolution to prevent false matches. Add non-localhost binding warning,
log writeJSON encoder errors, add ?permanent=true for explicit hard
delete, preserve title/description during absorb, use millisecond
backup timestamps, add path.Clean to spaHandler. Frontend gains
checkedJSON() for resp.ok validation, consistent stopPropagation, and
shared renderCardSections() to eliminate duplicate rendering.

CI / test (pull_request) Successful in 2m13s

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/code-review-findings:fix/code-review-findings

git checkout fix/code-review-findings

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lerko/nib-v1#46