Skip to content

fix(tasks): share mutation history and side effects - #1876

Merged
tinsever merged 5 commits into
fix/review-09-bulk-statusfrom
fix/review-10-mutation-parity
Sep 30, 2026
Merged

tinsever merged 5 commits into
fix/review-09-bulk-statusfrom
fix/review-10-mutation-parity

Conversation

@tinsever

@tinsever tinsever commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

what changed?

full, field-specific and bulk task updates disagreed about title history, assignment events and reminder resets. use shared mutation effects and emit the same field-change events after commit.

finding 10 from the codebase review.

based on #1873 (finding 09). merge that pr first; this diff contains only finding 10.

how did you check it?

postgres mutation parity tests covering title history, assignment/unassignment and due-date reminder reset.

checked the combined fix tree with the full workspace test run, uncached workspace typechecks, lint, i18n and openapi checks. postgres integration tests used disposable local test databases; provider and s3 calls were mocked.

Native GitHub stack #1886 (bottom to top): #1873 → #1876 (this PR) → #1877 → #1878 → #1879. Stack base: fix/review-06-integration-scope, the shared #1867 foundation. Merge #1867 first, then move this stack's base to main before merging board changes. If GitHub has not moved it automatically, unstack the still-open board PRs, retarget #1873 to main, and recreate this native stack in the same order.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T11:16:56.162388Z f03bf5f Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Mixed activity

Activity patterns show a mix of organic and automated signals.

View full analysis →

This is an automated analysis by AgentScan

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Align task mutation history, events, and reminder resets

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Share change detection across full, field-specific, and bulk task updates for consistent events.
• Record title history and reset due-date reminders transactionally when values change.
• Add PostgreSQL integration tests for full-update effects and bulk reminder behavior.
Diagram

graph TD
  Full["Full update"] --> Effects["Mutation effects"] --> Store[("History and reminders")]
  Field["Field update"] --> Effects --> Events["Task events"]
  Bulk["Bulk update"] --> Effects --> Mentions["Mention notices"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Transactional event outbox
  • ➕ Preserves events for retry if publishing fails after commit.
  • ➖ Requires an outbox table, dispatcher, and delivery semantics beyond this parity fix.

Recommendation: Use the shared-effects approach for this parity fix: it centralizes comparisons while keeping title history and reminder resets atomic with their updates. Consider an outbox separately if reliable delivery of post-commit events becomes a requirement.

Files changed (10) +418 / -292

Bug fix (4) +152 / -187
bulk-update-tasks.tsApply shared effects to bulk mutations +44/-68

Apply shared effects to bulk mutations

• Bulk status, priority, assignee, and due-date operations use shared change detection for events. Due-date updates now reset sent reminders in the same transaction as the task update.

apps/api/src/task/controllers/bulk-update-tasks.ts

update-task-due-date.tsReset reminders atomically on due-date changes +25/-22

Reset reminders atomically on due-date changes

• Locks the task and updates its due date alongside conditional reminder cleanup in a transaction. Publishes the field-change event afterward through the shared helper.

apps/api/src/task/controllers/update-task-due-date.ts

update-task-title.tsRecord title changes from locked task state +16/-18

Record title changes from locked task state

• Locks the task before updating it, then uses the shared helper to record changed-title history within the transaction. Publishes the title event afterward.

apps/api/src/task/controllers/update-task-title.ts

update-task.tsBring full updates into mutation parity +67/-79

Bring full updates into mutation parity

• Locks the task and records title history and due-date reminder cleanup within the update transaction. Shared post-update effects now cover field changes, including assignment, due date, description mentions, and asset cleanup.

apps/api/src/task/controllers/update-task.ts

Refactor (5) +170 / -105
task-mutation-effects.tsCentralize task mutation side effects +160/-0

Centralize task mutation side effects

• Adds transaction-scoped title history and changed-due-date reminder resets. A separate helper publishes changed-field events and handles description asset cleanup and newly mentioned users.

apps/api/src/task/controllers/task-mutation-effects.ts

update-task-assignee.tsUse shared assignment event handling +3/-34

Use shared assignment event handling

• Replaces local assignment and unassignment event construction with shared before/after mutation handling.

apps/api/src/task/controllers/update-task-assignee.ts

update-task-description.tsUse shared description effects +3/-45

Use shared description effects

• Delegates description events, orphaned-asset cleanup, and new-mention notifications to the shared helper.

apps/api/src/task/controllers/update-task-description.ts

update-task-priority.tsShare priority change detection +2/-10

Share priority change detection

• Replaces the controller's priority event payload with shared before/after event handling.

apps/api/src/task/controllers/update-task-priority.ts

update-task-status.tsShare status events and relation refresh +2/-16

Share status events and relation refresh

• Delegates changed-status events and task-relation refreshes to the shared mutation helper.

apps/api/src/task/controllers/update-task-status.ts

Tests (1) +96 / -0
task-mutation-parity.test.tsTest full-update and bulk due-date parity +96/-0

Test full-update and bulk due-date parity

• Adds PostgreSQL integration tests for full-update title history, assignment events, and reminder resets. Verifies bulk updates retain reminder claims for unchanged dates and clear them when the date is removed.

tests/api-integration/task-mutation-parity.test.ts

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12ace3b452

ℹ️ 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".

Comment thread apps/api/src/task/controllers/bulk-update-tasks.ts Outdated
Comment thread apps/api/src/task/controllers/bulk-update-tasks.ts Outdated
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Bulk status changes repeat board refreshes ✓ Resolved
Description
publishTaskMutation emits task-relation.refresh for every status change, and bulkUpdateTasks
calls it once per updated task instead of retaining one refresh per project. When multiple tasks in
a project change status, each event runs the WebSocket refresh handler and triggers a project-wide
relation update, even though outbound messages are later coalesced; clients can repeatedly
invalidate the project's tasks and relations.
Code

apps/api/src/task/controllers/task-mutation-effects.ts[R73-76]

+    await publishEvent("task-relation.refresh", {
+      projectId: after.projectId,
+      userId,
+    });
Evidence
The bulk status loop calls the shared publisher for each updated task, and the publisher emits a
project-scoped relation refresh for each status change. The WebSocket subscriber handles every event
and calls broadcastToProject; its timed queue coalesces outbound messages only after those handler
calls. The resulting project-wide relation update causes the web client to invalidate the project's
tasks and task relations.

AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient: AGENTS.md: Keep Task-Heavy Boards and Realtime Updates Efficient
apps/api/src/task/controllers/task-mutation-effects.ts[64-76]
apps/api/src/task/controllers/bulk-update-tasks.ts[154-161]
apps/api/src/ws/index.ts[421-439]
apps/web/src/hooks/use-project-websocket.ts[78-103]
apps/api/src/task/controllers/bulk-update-tasks.ts[153-163]
apps/api/src/task/controllers/task-mutation-effects.ts[64-77]
apps/api/src/ws/index.ts[424-442]
apps/api/src/ws/index.ts[279-319]
apps/api/src/task/controllers/bulk-update-tasks.ts[151-161]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Bulk status updates emit a project-wide relation refresh for every changed task rather than once per affected project. Each event still invokes the WebSocket handler, even when outbound messages are coalesced.
## Fix Focus Areas
- apps/api/src/task/controllers/task-mutation-effects.ts[56-77]
- apps/api/src/task/controllers/bulk-update-tasks.ts[150-163]
- apps/api/src/ws/index.ts[424-442]
## Recommended Fix
Add an option to the shared mutation publisher that suppresses its per-task `task-relation.refresh` event. Use it in bulk status updates while preserving individual status events, collect distinct project IDs containing changed tasks, and publish one relation refresh per affected project after the updates commit. Keep the existing refresh behavior for individual status changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Bulk date changes can miss reminders ✓ Resolved
Description
bulkUpdateTasks passes due dates read before its transaction to recordTaskMutation, rather than
reading each task's current date under a lock. If another update changes a date between that read
and the bulk write, the comparison can leave a reminder claim in place for the newly written date
and suppress the due-date event.
Code

apps/api/src/task/controllers/bulk-update-tasks.ts[R372-373]

+        for (const task of tasks)
+          await recordTaskMutation(tx, task, { dueDate: parsedDate }, userId);
Evidence
The initial read precedes the transaction, while the new comparison and event publication both use
that snapshot. Reminder selection excludes tasks with an existing claim, making an incorrectly
preserved claim consequential.

apps/api/src/task/controllers/bulk-update-tasks.ts[42-56]
apps/api/src/task/controllers/bulk-update-tasks.ts[363-382]
apps/api/src/task/controllers/task-mutation-effects.ts[42-49]
apps/api/src/scheduler/due-date-reminders.ts[70-85]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Bulk due-date updates compare against task dates captured before the transaction, so concurrent changes can leave reminder claims in place and omit events.
## Fix Focus Areas
- apps/api/src/task/controllers/bulk-update-tasks.ts[363-382]
## Recommended Fix
Read and lock the affected rows inside the transaction before updating them. Use those locked pre-update values for both `recordTaskMutation` and post-commit event publication.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Concurrent edits create false task events ✓ Resolved
Description
publishTaskMutation compares every field in the pre-update snapshot with the returned row, while
single-field controllers such as updateTaskPriority, updateTaskStatus, updateTaskAssignee, and
updateTaskDescription obtain that snapshot before an unlocked update. When another request changes
a different field between those operations, this helper republishes that unrelated change under the
single-field request, reaching activity, notification, plugin, and WebSocket subscribers with
duplicate or incorrectly attributed events.
Code

apps/api/src/task/controllers/task-mutation-effects.ts[R64-67]

+  if (before.status !== after.status) {
+    await publishEvent("task.status_changed", {
+      ...common,
+      oldStatus: before.status,
Evidence
The helper emits status, title, priority, assignee, due-date, and description events solely from
before/after differences. The narrow controllers read existingTask, then run a separate unlocked
update, so PostgreSQL can return a row containing a concurrent change that the current controller
did not make; activity and notification subscribers act on these emitted event payloads.

apps/api/src/task/controllers/task-mutation-effects.ts[64-129]
apps/api/src/task/controllers/update-task-priority.ts[15-38]
apps/api/src/task/controllers/update-task-status.ts[16-48]
apps/api/src/task/controllers/update-task-assignee.ts[20-54]
apps/api/src/task/controllers/update-task-description.ts[15-38]
apps/api/src/activity/index.ts[226-300]
apps/api/src/notification/index.ts[181-256]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The shared publisher infers every changed field by diffing a potentially stale pre-update snapshot against the returned row. Narrow controllers do not lock the row, so a concurrent mutation to a different field can cause the current request to emit duplicate events for that other mutation.
## Fix Focus Areas
- apps/api/src/task/controllers/task-mutation-effects.ts[52-119]
- apps/api/src/task/controllers/update-task-priority.ts[15-38]
- apps/api/src/task/controllers/update-task-status.ts[16-48]
- apps/api/src/task/controllers/update-task-assignee.ts[20-54]
- apps/api/src/task/controllers/update-task-description.ts[15-38]
## Recommended Fix
Change the shared publisher to accept the set of fields intentionally mutated by the caller and only compare/publish those fields. Pass the relevant single field from each narrow controller and the explicit field set from bulk operations; retain the full field set for the locked full-update path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. Bulk assignment repeats assignee lookups ✓ Resolved
Description
publishTaskMutation queries the assignee's name on every assignment change, and bulkUpdateTasks
calls it separately for each selected task. Assigning many tasks to one user now performs a database
lookup per task instead of the single lookup made by the previous bulk path.
Code

apps/api/src/task/controllers/bulk-update-tasks.ts[R204-207]

+      for (const task of tasks)
+        await publishTaskMutation(
+          task,
+          {
Evidence
The bulk loop calls the helper for each task; whenever its assignee differs, the helper executes a
separate user-table query for the same target ID. The bulk request schema has no maximum task count.

apps/api/src/task/controllers/bulk-update-tasks.ts[199-213]
apps/api/src/task/controllers/task-mutation-effects.ts[92-111]
apps/api/src/task/schema.ts[51-53]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Bulk assignment performs one redundant assignee-name query per changed task.
## Fix Focus Areas
- apps/api/src/task/controllers/bulk-update-tasks.ts[199-213]
- apps/api/src/task/controllers/task-mutation-effects.ts[92-111]
## Recommended Fix
Resolve the new assignee name once for a bulk operation and pass it to the shared publisher, while retaining its lookup for single-task callers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread apps/api/src/task/controllers/task-mutation-effects.ts Outdated
Comment thread apps/api/src/task/controllers/bulk-update-tasks.ts Outdated
Comment thread apps/api/src/task/controllers/bulk-update-tasks.ts Outdated
Comment thread apps/api/src/task/controllers/task-mutation-effects.ts Outdated
@tinsever

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8f2edce3fc

ℹ️ 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".

Comment thread apps/api/src/task/controllers/task-mutation-effects.ts
@tinsever

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43e0b37953

ℹ️ 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".

Comment thread apps/api/src/task/controllers/update-task-status.ts Outdated
@tinsever

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: df36d9eca8

ℹ️ 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".

@tinsever

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f03bf5f67f

ℹ️ 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".

Comment on lines +45 to 47
await publishTaskMutation(existingTask, updatedTask, currentUserId, {
fields: ["status"],
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Publish updates for column-only status repairs

When a task already has the requested status but its columnId is null or stale, this call emits nothing because publishTaskMutation compares only status, even though the preceding update changes columnId. This occurs for existing Gitea/GitLab imports that store a status without a column ID, leaving other clients' board caches stale; the bulk status path already handles this case with a task.updated fallback and relation refresh. Include columnId in the change detection or publish the same fallback here.

AGENTS.md reference: AGENTS.md:L32-L32

Useful? React with 👍 / 👎.

@tinsever
tinsever added this pull request to stack #1886 September 30, 2026 11:20
@tinsever
tinsever removed this pull request from stack #1886 September 30, 2026 16:59
@tinsever
tinsever added this pull request to stack #1888 September 30, 2026 16:59
@tinsever
tinsever merged commit e25f53a into main Sep 30, 2026
24 checks passed
@tinsever
tinsever deleted the fix/review-10-mutation-parity branch September 30, 2026 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant