Repository navigation
feat: the task tools can list overdue tasks, with the days overdue EXO-91122 - #669
Conversation
|
Source
Picks on this branch ( Declared commit: Adaptations: no conflict hunk, no POM change; one declared commit, below. Verification (head
Landing: one task, squash. Level: N1, as the source PR's: the permission checks of |
ahamdi
left a comment
There was a problem hiding this comment.
AI review — Round #1
🟡 Medium — the PR body's test run and classification are stated at d71567872, which this PR does not contain
mvn -pl services -am test -Dtest='TestTaskDAO,TaskMcpToolTest'atd71567872: 25 + 66 tests, 0 failures.
This change is classified N1, computed atd71567872.
d71567872 is the last feature/ai-contribution commit of task#667; this branch carries its pick efa067732, which did not compile on develop until the declared commit ebac71b79. At this head TaskMcpToolTest has 65 @Test methods, not 66 (createPersonalTaskShouldAssignTheCallerWhenNoAssigneeIsGiven exists only on the FB), and TaskMcpTool / ai-tool-definitions.json also differ from d71567872. The body is the squash commit's message on develop, so it would land a run on a SHA outside develop's history with a count that is wrong for the code it describes (pr-conventions.md §5). The evidence comment already has a run at the head (mvn -B verify -Pcoverage at ebac71b79, 238 tests).
Fix: state the run at ebac71b79 in the body — the targeted command's result line at this head (25 + 65), or the evidence comment's full-reactor line — and write the classification as computed at ebac71b79.
🟢 Nit — the evidence comment's Picks line lists 42e194c69 … efa067732 as the "FB commits"; those are this branch's picks. Their sources (the -x trailers) are f2cd923e8 ea5e344c4 c2915f79a 67a2a3919 0cd75225c d71567872, all on feature/ai-contribution. The patch-ids do match one to one (6/6, recomputed), so only the label is off.
Verified conform
- Pick equivalence: each of the six picks has the same
git patch-id --stableas itsfeature/ai-contributionsource; the only other commit is the declared one-line importebac71b79(test code). There are no conflict hunks. - ACL on
list_tasks:getTaskQuerykeepsprojectService.getProject→EntityNotFoundException→ObjectNotFoundExceptionandproject.canView(aclIdentity)→IllegalAccessExceptionfor a project, andsetAccessible(aclIdentity)without one. The list and the count now share that one query, so the count cannot select tasks the list does not.countTaskModelsandgetTasksgo through the same path. list_assigned_tasksoverdue scope:setAssigneeOrCoworkerbuildsassignee IN (…) OR coworker IN (…).TaskDAOImpl#buildPathjoinscoworkerwithJoinType.LEFT, so a task with no coworker still matches on its assignee.findEntitiesselectsdistinctandbuildCountQueryusescountDistinct. This is the scope of theTask.getUncompletedTasksnamed query.- Due date semantics:
setOverdueAtfilters ondueDate(lt, strict) pluscompleted = false, neverendDate. This is the date the UI binds, so theenddate-column-is-not-the-ui-due-datepitfall does not apply. The cut-off is the start of the user's day in their profile zone (McpToolUtils.getUserTimeZone), with the server's zone as fallback. - Ordering:
CommonJPADAO#findEntitiesapplies thedueDate ASC, id ASCOrderBylist.testFindOverdueTasksMostOverdueFirstAcrossPagesruns it through JPA/HSQLDB over two pages, with identifiers that run against the due dates, so only the due date order passes. - Tool binding:
McpToolCallbackProviderServicebinds the tool arguments by name (snake → camel), so addingoverdueOnlybeforeoffsetinlistTasksdoes not shift any argument. No caller inaiorprocessesuses the changed Java signatures.annotationsandrequire_approvalare unchanged. Knowledge:: Meeds-io/eng-standards#382 is open and a draft, stacked on #362 (ku/pitfalls-eva-orchestration). It carries aknowledge/pitfalls/task/file and its change fragment for this delivery.- There is no schema, i18n or REST surface in the diff.
The PR is good work. The new query criterion is pinned through the real engine (lt vs lte, a task due today, a completed task, no due date, another user's task). The DST and skipped-midnight cases are checked against a day-by-day count. The count of list_tasks now follows the filters it lists. The tool descriptions tell the agent how to reach the rest of a truncated, most-overdue-first page. The known limit on time-less due dates is written down instead of being left hidden.
Classification: N1. The permission checks of list_tasks (canView, setAccessible) move into the shared getTaskQuery, and the input schemas of two existing MCP tools change. An Architect or Developer who knows it is N1 must validate this PR, and it must not be merged on the AI review alone; author ≠ approver.
🤖 Generated with Claude Code
|
Round 1, both taken; no commit, the head stays
|
ahamdi
left a comment
There was a problem hiding this comment.
AI review — Round #2 (final)
| Finding (Round #1) | Status |
|---|---|
🟡 The PR body's test run and classification were stated at d71567872 |
✅ Fixed: the live body now gives mvn -B verify -Pcoverage at ebac71b79 (238 tests, 0 failures) and the classification as computed at ebac71b79. No SHA outside develop's history remains. |
| 🟢 The evidence comment labelled the picks as "FB commits" | ✅ Fixed: the Picks line now names this branch's six picks and their feature/ai-contribution sources f2cd923e8 … d71567872. |
No commit since Round #1: the head is still ebac71b79, so the code and the conform points verified in Round #1 still hold.
All findings from previous rounds are resolved. Nothing is outstanding from the AI review side.
Classification: N1. The permission checks of list_tasks (canView, setAccessible) move into the shared getTaskQuery, and the input schemas of two existing MCP tools change. An Architect or Developer who knows it is N1 must validate this PR, and it must not be merged on the AI review alone; author ≠ approver.
🤖 Generated with Claude Code
…O-91122 Asked what was late, a small model compared due dates with today across many tasks and missed a task ten days overdue; that task was a personal one, which no per-project list shows. list_tasks and list_assigned_tasks take overdue_only: the open tasks whose due date (the one the Tasks UI shows, never the end date) is before the start of today in the user's time zone; a task due today is not overdue. list_tasks without a project already spans every task the user can access, personal tasks included, so no separate list_overdue_tasks tool: one more tool would only duplicate that query in the catalogue. Every open task past its due date now carries days_overdue, counted in whole days in the user's time zone. The condition lives on TaskQuery (setOverdueAt) and is tested through the engine. list_tasks counted every task of its scope whatever its filters; its count is now the count of the query it lists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f2cd923)
A test asserts the user scope of the moved queries on both tools, and the engine test leaves another user's overdue task out. The count of the project activity tool reuses the one query builder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit ea5e344)
… how to page EXO-91122 The overdue_only listings were cut to one page ordered by last activity. They now order by due date ascending (ties on the identifier); the change log and unscheduled listings reuse the user's time zone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 67a2a39)
…s, so only the due date order passes EXO-91122 The tasks were created from the most overdue to the least, so the identifier order gave the same pages as the due date order: a listing ordered by identifier, or by no order at all, passed. They are now created from the least overdue to the most, the tie first, and the Javadoc counts sixteen tasks and a second page of six. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 0cd7522)
…EXO-91122 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d715678)
…op's copy of the test lacked EXO-91122 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ebac71b to
12f771d
Compare
|



Why: an AI agent asked "what's late?" had to fetch every task and compare due dates itself, and missed overdue tasks when it didn't know today's date (EXO-91122). The task tools now answer it directly. The TASK specialist of Meeds-io/ai#351 (EXO-91055) calls
list_taskswithoverdue_only, so this PR merges before it.How:
TaskQuery#setOverdueAt(startOfToday): open tasks whosedueDate(the date the Tasks UI shows, neverendDate) is strictly before the start of the user's day. A task due today, a completed task and a task with no due date are not overdue.list_tasksandlist_assigned_taskstake anoverdue_onlyboolean (ai-tool-definitions.json).annotationsandrequire_approvalare unchanged. The cut-off is today in the time zone of the user's profile (McpToolUtils.getUserTimeZone), the server's when the profile has none.days_overdue(TaskModel), in whole days in that time zone.TaskMcpTool#getTaskQuerybuilds the query oflist_tasksfor both the list and its count:project.canView, or every task the user can access (setAccessible), personal tasks included, as before;hide_completed_tasksandoverdue_onlyincluded, where it used to count every task.list_assigned_tasksselects the user's tasks as assignee or coworker, the same scope asTask.getUncompletedTasks.TaskService#findTasks), so a truncated page keeps the worst ones. Their descriptions say how to get the rest:list_taskspages withoffsetwhilecountis above the tasks received;list_assigned_tasks, which has nooffset, takes a largerlimit.Tests:
TestTaskDAO#testFindOverdueTasksBeforeTheStartOfTheUsersDayruns the criterion through JPA on HSQLDB.TestTaskDAO#testFindOverdueTasksMostOverdueFirstAcrossPagesruns the order through JPA over two pages, with tasks created against their due dates so that only the due date order passes.TaskMcpToolTestpins the queries, the counts,days_overduearound midnight across time zones, and the profile time zone.lt→lte, the open-task condition removed, a task due today counted, the server's zone always used, the count of every task; the overdue order inverted or removed in the tool; noORDER BY, and ascending turned descending, inCommonJPADAO.daysOverdueAcrossTimeZoneTransitionsMatchADayByDayCountandtheOverdueCutOffIsTheStartOfTheDayWhenTheZoneSkipsMidnightpindays_overdueand the cut-off across daylight-saving changes and a skipped midnight, against a day-by-day count.mvn -B verify -Pcoverageatebac71b79ondevelop37688705f(full reactor, no install): BUILD SUCCESS, 238 tests, 0 failures, coverage checks met.Known limit: a due date given without a time through the MCP task tools was observed stored at midnight in the server's time zone (EXO-91055 testing; not re-verified in this PR, and not checked for the Tasks UI date picker), while the cut-off here is the start of the user's day. For a user west of the server's zone, such a task is counted overdue on its due day itself, and its
days_overdueis one more; east of it the day is right. That storage behaviour predates this PR.Knowledge: Meeds-io/eng-standards#382 (draft, stacked on #362, until this change reaches
develop).This change is classified N1, computed at
ebac71b79. The permission checks oflist_tasks(canView,setAccessible) move into onegetTaskQueryshared by the list and the count, andlist_assigned_tasksgains its own user-scoped query. Its approver must be an Archi/Dev who knows it is N1, not an approval on AI review alone; author ≠ approver.Backport of #667 from
feature/ai-contribution, picked as it is, plus one declared commit (ebac71b79: the test imports the captor it uses, whichdevelop's copy ofTaskMcpToolTestlacked). The reviewer's evidence (source, picks, patch-ids, build) is a PR comment.🤖 Generated with Claude Code