Skip to content

Address review findings cleanup#7

Merged
ryan-wong-coder merged 1 commit into
mainfrom
issue-6-review-findings
Apr 28, 2026
Merged

Address review findings cleanup#7
ryan-wong-coder merged 1 commit into
mainfrom
issue-6-review-findings

Conversation

@ryan-wong-coder

@ryan-wong-coder ryan-wong-coder commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Fixes #6

What changed

  • Confirmed the integration timing fix from the previous CI-quality work and verified the full integration suite with low parallelism.
  • Kept WAL startup recovery on the streaming replay path and cleaned a stale comment that still described the old collection-based loader.
  • Removed the production-only appearance of full-leaf global consistency proof generation; the full-leaf reference helper now lives in tests only.
  • Clarified the batch commit hook contract: it may enqueue a durable local outbox event, while global append and notary work stay in the outbox worker.
  • Removed legacy backup archive helpers that read whole entries into memory; streaming verify/restore remains the only implementation path.
  • Added a desktop regression test ensuring Wails-exposed record DTOs do not contain time.Time fields.

Validation

  • go test ./...
  • go vet ./...
  • go test -p 1 -count=1 -tags=integration ./...
  • go test -p 1 -count=1 -tags=e2e ./...
  • go test -p 1 -race ./...
  • go test -p 1 -race -tags="integration e2e" ./...
  • cd clients/desktop && go test ./...
  • cd clients/desktop && go test -race ./...
  • cd clients/desktop && go vet ./...
  • cd clients/desktop/frontend && npm run build
  • cd clients/desktop && wails build
  • git diff --check

Note: Cursor Bugbot is intentionally not treated as a blocking TrustDB CI signal for this PR.


Note

Low Risk
Low risk: changes are limited to tests, comment/contract clarifications, and removal of unused backup/global-log helper code with no behavioral changes in production paths.

Overview
Adds a desktop regression test that reflect-walks Wails-exposed record DTOs to ensure they don’t contain time.Time fields.

Cleans up stale/ambiguous documentation by clarifying WAL replay segment-skipping behavior and tightening the OnBatchCommitted hook contract to only allow bounded local side effects (e.g., durable outbox enqueue) while keeping global append/anchoring out of the batch worker.

Removes unused legacy backup-archive helpers that read whole entries into memory, and moves the global-log full-leaf hash scan helper out of production code into a test-only reference helper.

Reviewed by Cursor Bugbot for commit 2deeaec. Bugbot is set up for automated code reviews on this repo. Configure here.

@ryan-wong-coder
ryan-wong-coder merged commit 185abd4 into main Apr 28, 2026
8 checks passed
@ryan-wong-coder
ryan-wong-coder deleted the issue-6-review-findings branch April 28, 2026 13:13

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2deeaec. Configure here.

t.Fatalf("%s.%s exposes time.Time to Wails bindings", typ, field.Name)
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DTO guard test doesn't recurse into nested structs

Medium Severity

The assertNoTimeTimeFields function doesn't recursively inspect nested structs, only checking immediate fields after unwrapping pointers/slices. This allows time.Time fields in transitively embedded structs (like those within LocalRecord) to be missed, providing false confidence that Wails bindings are free of time.Time types.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2deeaec. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

收敛 review findings:恢复、global log、batch hook、backup、Wails DTO

2 participants