fix: keep Temporal workflow when a scheduled post is edited without a time change#1690
fix: keep Temporal workflow when a scheduled post is edited without a time change#1690giladresisi wants to merge 4 commits into
Conversation
… time change Editing a future scheduled post (dashboard or public API) sent type 'schedule', which always terminated and recreated the post's Temporal workflow - even when only content or settings changed and the publish time stayed the same. At 100k+ sleeping workflows that is needless churn, and it diverges from the agent's updatePostTool, which already leaves the workflow alone for non-time changes. Decide it server-side in createPost: capture the post's current publish time before the upsert, and only (re)start the workflow when it's a new post or an existing QUEUE post whose time actually moved. Content and settings are re-read from the DB at publish time, so an unchanged time needs no restart; a genuine time change still restarts it. Done in the service rather than the edit modal so it's authoritative and covers every caller (dashboard, public API), and can't be fooled by client-side date state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Contribution-checker quality warning Heuristics that flagged:
If this is a genuine contribution, please add detail to your PR description and tighten the diff scope before reviewers look at it. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| content: removeLinks ? stripLinks(updateContent[i]) : updateContent[i], | ||
| })); | ||
|
|
||
| const scheduledDate = |
There was a problem hiding this comment.
Bug: On non-UTC servers, creating a scheduledDate with dayjs().format() and parsing it with dayjs.utc() causes an incorrect time comparison for posts with type: 'now'.
Severity: MEDIUM
Suggested Fix
Ensure timezone consistency. When generating the scheduledDate for type === 'now', use dayjs.utc() to create a UTC-based time string. Replace dayjs().format('YYYY-MM-DDTHH:mm:00') with dayjs.utc().format('YYYY-MM-DDTHH:mm:00') to align the generated time with the UTC-based comparison logic used later.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: libraries/nestjs-libraries/src/database/prisma/posts/posts.service.ts#L903
Potential issue: A timezone mismatch occurs when editing a post with `body.type ===
'now'` on a server not running in the UTC timezone. The code generates a `scheduledDate`
using `dayjs().format()`, which creates a string representing the server's local time
but lacks a timezone offset. This string is later parsed by `dayjs.utc()`, which
incorrectly assumes the time is in UTC. This discrepancy causes the time comparison
against the existing `publishDate` to fail, leading to an unnecessary workflow restart
even when the scheduled time has not been modified.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Fixed with the new commit
…ish times createOrUpdatePost persists publishDate with a local-time parse (dayjs(date).toDate()), but the unchanged-time guard compared with dayjs.utc(scheduledDate). On non-UTC servers the guard was off by the server offset: usually failing open (needless workflow restart), and wrongly skipping the restart when a time moved by exactly that offset. Parsing both sides the way the repository does makes the guard exact on any server timezone; on UTC servers behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Contribution-checker quality warning Heuristics that flagged:
If this is a genuine contribution, please add detail to your PR description and tighten the diff scope before reviewers look at it. |
| const publishTimeUnchanged = | ||
| !!existingRoot && | ||
| existingRoot.state === 'QUEUE' && | ||
| dayjs(existingRoot.publishDate).isSame(dayjs(scheduledDate), 'minute'); | ||
|
|
||
| if (body.type !== 'update' && !publishTimeUnchanged) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed in a followup commit
… posted now publishTimeUnchanged only looked at the pre-update state, so saving a queued post as a draft (or posting it now) with an unchanged time skipped startWorkflow and left the old Temporal workflow running, which would publish the draft at its original time. Gate the skip on body.type === 'schedule' so only queued-stays-queued edits keep the existing workflow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| const existingRootId = post.value?.[0]?.id; | ||
| const existingRoot = existingRootId | ||
| ? await this._postRepository.getPostById(existingRootId, orgId) | ||
| : null; | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Partially right — normal group edits update rows in place via upsert (ids preserved), so no new records are created and the existing workflow stays keyed to the same root; and when records genuinely are recreated, existingRoot is null so the skip can't fire. The real gap was an edit that promotes a child row to root (thread head removed/reordered) — fixed in 043e2da by requiring existingRoot to have no parentPostId.
If an edit removes or reorders the thread head, value[0] is a promoted child row that passes the QUEUE/same-time checks, while the workflow is keyed to the old root, which gets soft-deleted by the group cleanup. Require existingRoot to have no parentPostId so such edits start a workflow for the new root. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
Editing a future scheduled post — via the dashboard or the public API (
POST /public/v1/postswith the existing ids) — senttype: 'schedule', which always terminated and recreated the post's Temporal workflow, even when only the content or settings changed and the publish time stayed the same. At 100k+ sleeping workflows that is needless churn (terminate + start per edit), and it diverged from the agent'supdatePostTool, which already leaves the workflow alone for non-time changes.Fix
Decide it server-side in
PostsService.createPost: capture the post's current publish time before the upsert overwrites it, and only (re)start the workflow when it's a new post or an existingQUEUEpost whose time actually moved. Content and settings are re-read from the DB at publish time, so an unchanged time needs no restart; a genuine time change still restarts it.Done in the service (not the edit modal) so it's authoritative and covers every caller — dashboard and public API — and can't be fooled by client-side date state.
Testing (repro + validate, both surfaces)
Counted Temporal runs for the post before/after an identical time-unchanged edit:
POST /public/v1/posts(same ids, same time)A real date change still recreates the workflow (
TERMINATE_EXISTING, one new run) in both cases. Decision logic also spot-checked against real stored dates (unchanged → skip, moved → restart).🤖 Generated with Claude Code