Code review Round 3: state sync fixes in undo flows#7
Conversation
Verified all Round 2 critical fixes (CR-1 through CR-4) are correctly implemented. Found 4 important issues around stale task state after "Done today" undo operations and Today's 5 snapshot management. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- I-9: markWorkedOn/unmarkWorkedOn now call _refreshCurrentList() instead of notifyListeners(), fixing stale grid after undo on already-started tasks - I-10: Today's 5 _workedOnTask re-fetches task snapshot after mutation - I-11: Today's 5 _completeNormalTask undo re-fetches task so navigate button reappears correctly - I-12: Today's 5 "Done today" now has undo support matching All Tasks - M-11: Consolidate triple _refreshCurrentList() into single batch method markWorkedOnAndNavigateBack() - Update CLAUDE.md: fix commands can run in any session except the corresponding review session Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01255cf6a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| action: SnackBarAction( | ||
| label: 'Undo', | ||
| onPressed: () async { | ||
| await provider.unmarkWorkedOn(task.id!); |
There was a problem hiding this comment.
The new "Done today" undo handler calls unmarkWorkedOn without restoreTo, which causes last_worked_at to be set to NULL instead of restoring the task's previous value. This means if a task already had historical lastWorkedAt data (for example from yesterday), doing "Done today" and then Undo silently erases that history and changes later weighting/sorting behavior; the All Tasks flow avoids this by capturing and restoring the prior timestamp.
Useful? React with 👍 / 👎.
Security fixes: - MED-6: Brain dump dialog: maxLength 25000, per-line truncation to 500 chars - LOW-4: Pre-import backup (.db.bak) before overwriting existing database - LOW-9: Gradle warns when key.properties missing (debug signing fallback) - LOW-10: firstWhere with orElse in deleteTask/completeTask/skipTask - LOW-12: URL text field maxLength 2048 - LOW-13: normalizeUrl validates host contains a dot (rejects garbage input) Bug fix (from PR #7 review comment): - Today's 5 "Done today" undo now passes restoreTo to preserve historical lastWorkedAt instead of erasing it to NULL Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security review Round 2: fixes + PR #7 bug fix
1 participant
Summary
markWorkedOn/unmarkWorkedOnnow call_refreshCurrentList()instead ofnotifyListeners(), fixing stale grid after undo on already-started tasks_workedOnTaskre-fetches task snapshot after mutation for consistent UI state_refreshCurrentList()into singlemarkWorkedOnAndNavigateBack()batch methodTest plan
flutter test)flutter build linux)./dev.sh: mark task "Done today" in Today's 5, verify undo works🤖 Generated with Claude Code