fix: section 3 review fixes — git timeout, picker create-row, doc/measure items #28

Merged

fiatcode merged 5 commits from fix/review-notes-optimization into main

2026-06-18 17:34:02 +07:00

fiatcode commented

2026-06-18 17:30:51 +07:00

Owner

Section 3 of the code-review fixes — five small, independent items across internal/sync/, internal/edit/, internal/views/. Executed via subagent-driven-development; go vet ./... + full go test ./... green; final whole-branch review (opus) returned ready-to-merge with no Critical/Important findings.

Changes

fix(sync): bound every git command with a per-command context.WithTimeout (syncTimeout, default 2m, overridable for tests) so a hung git push can't leak the async sync goroutine or wedge the syncing single-flight flag forever. New TestRunTimesOutOnSlowGit (mutation-resistant — verified the test fails if the context isn't wired into the command).
feat(picker): offer the ＋ Create row whenever the exact name doesn't Resolve, even if fuzzy matches exist — fixes a dead-end where a name like Notes (fuzzy-matching Nested Notes Archive) could never be created. (UX decision: Option B over keeping current behavior.) One golden updated.
docs(edit): clarify WriteFile preserves rwx permission bits only — setuid/setgid/sticky dropped, owner/group reassigned to the weft process user.
docs(sync): explain why Result.Pulled's HEAD-diff is sufficient (logic confirmed sound, not a defect — comment only).
test(editor): benchmark View(); the proposed re-tint/dirty memoization was closed after measurement — 93% of cost is bubbles/textarea's own ta.View() which the cache can't avoid, and it misses on every keystroke. Benchmark retained as a guard.

Verification

go vet ./... clean; go test ./... green (7 packages).
git-timeout test mutation-checked (build-revert + faithful runtime mutation both fail; restored green).
Per-task reviews (Tasks 1, 3) spec ✅ / approved; final whole-branch review ready-to-merge.

Diffstat: 7 files, +65 / −10.

Section 3 of the code-review fixes — five small, independent items across `internal/sync/`, `internal/edit/`, `internal/views/`. Executed via subagent-driven-development; `go vet ./...` + full `go test ./...` green; final whole-branch review (opus) returned ready-to-merge with no Critical/Important findings. ## Changes - **fix(sync):** bound every git command with a per-command `context.WithTimeout` (`syncTimeout`, default 2m, overridable for tests) so a hung `git push` can't leak the async sync goroutine or wedge the `syncing` single-flight flag forever. New `TestRunTimesOutOnSlowGit` (mutation-resistant — verified the test fails if the context isn't wired into the command). - **feat(picker):** offer the `＋ Create` row whenever the exact name doesn't `Resolve`, even if fuzzy matches exist — fixes a dead-end where a name like `Notes` (fuzzy-matching `Nested Notes Archive`) could never be created. *(UX decision: Option B over keeping current behavior.)* One golden updated. - **docs(edit):** clarify `WriteFile` preserves rwx permission **bits** only — setuid/setgid/sticky dropped, owner/group reassigned to the weft process user. - **docs(sync):** explain why `Result.Pulled`'s HEAD-diff is sufficient (logic confirmed sound, not a defect — comment only). - **test(editor):** benchmark `View()`; the proposed re-tint/dirty memoization was **closed** after measurement — 93% of cost is bubbles/textarea's own `ta.View()` which the cache can't avoid, and it misses on every keystroke. Benchmark retained as a guard. ## Verification - `go vet ./...` clean; `go test ./...` green (7 packages). - git-timeout test mutation-checked (build-revert + faithful runtime mutation both fail; restored green). - Per-task reviews (Tasks 1, 3) spec ✅ / approved; final whole-branch review ready-to-merge. Diffstat: 7 files, +65 / −10.

fiatcode added 5 commits

2026-06-18 17:30:51 +07:00

fix(sync): bound git commands with a timeout so a hung push can't hang sync 4673772394

docs(edit): clarify WriteFile preserves permission bits only, not owner/special bits af8954fbdd

test(editor): benchmark View() on a large page 017d9dc54a

Measurement gate for the re-tint/dirty memoization idea. View() is 12.3ms on
a 2000-line page, but ~93% is bubbles/textarea's own ta.View() (O(buffer));
the tint + dirty steps the proposed cache would memoize are <1ms combined, and
on the keystroke hot path both cache keys change every frame (cache miss). The
cache can't skip ta.View() since it must call it to build the key. Realistic
pages (<200 lines) already render in ~1.5ms. Closing the optimization per YAGNI;
benchmark retained as a regression guard.

docs(sync): explain why Result.Pulled HEAD-diff is sufficient 49c7193d74

feat(picker): offer create row whenever the exact name doesn't resolve

CI / vet-and-test (pull_request) Successful in 27s

Details

61d9ae9f06