fix: sync kanban card stage with live agent phase#1890
fix: sync kanban card stage with live agent phase#1890
Conversation
The emitPhaseFromState method in TaskStateManager was using Date.now() as the sequenceNumber for TASK_EXECUTION_PROGRESS events. Since agent processes use small sequential integers (1, 2, 3...) for their sequence numbers, the timestamp-based value (e.g., 1708512345678) was always much larger. This caused the out-of-order drop check in updateExecutionProgress to permanently block all subsequent agent execution-progress events once XState emitted any phase update. The result: after XState fired the initial "planning" phase via emitPhaseFromState, all agent events reporting the "coding" (or later) phase would be silently dropped because their sequential numbers were less than the stored timestamp. The kanban card would remain frozen on the "planning" badge while the task detail view (which reads from task logs) correctly showed the "coding" stage. Fix: use sequenceNumber=0 in emitPhaseFromState. With value 0, the incoming sequence check (incomingSeq > 0 && currentSeq > 0) is not triggered, so the XState phase update always applies. Subsequent agent events with their small sequential numbers are then correctly accepted and can continue updating phaseProgress. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @AndyMik90, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical synchronization bug in the frontend that prevented kanban board task cards from accurately reflecting the live agent's progress. By correcting the sequence numbering logic for phase transition events, the changes ensure that the UI consistently displays the current state of tasks, improving the user experience and reliability of the task management interface. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
area/frontend
📝 WalkthroughWalkthroughemitPhaseFromState now sets executionProgress.sequenceNumber to 0 and zeroes phase/overall progress for non-final phases; comments explain sequencing rationale and behavior. No exported signatures changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug causing the Kanban card's stage to become stuck. The root cause was correctly identified: using Date.now() as a sequence number for XState-driven phase updates was preventing subsequent agent progress events from being processed. The fix, which changes the sequence number to 0, is a clean and effective solution. The new comment added to explain the rationale is excellent and greatly improves the maintainability of the code. The change is well-targeted and appears correct.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
PR #1890 reviewed: 8 findings (1 structural issues). Verdict: needs_review.
Findings (8 selected of 8 total)
🟠 [QLT-1] [HIGH] sequenceNumber: 0 allows out-of-order delivery in reverse direction
📁 apps/frontend/src/main/task-state-manager.ts:380
While changing from Date.now() to 0 fixes the original bug where large timestamps poisoned the sequence counter, it introduces a new edge case: if emitPhaseFromState is called after the agent has already emitted events with sequenceNumber >= 1, the XState-derived phase update (with sequenceNumber 0) will be silently dropped by the same out-of-order guard (incomingSeq < currentSeq). This means late XState transitions (e.g., transitioning to 'complete') may never reach the renderer if agent events have already incremented the counter. A more robust fix would use a separate channel/flag to bypass the sequence guard for XState-sourced events, or use a dedicated sequence counter that doesn't collide with the agent's counter space.
Suggested fix:
Consider adding a `bypassSequenceCheck: true` flag to XState-sourced events and updating the renderer's guard logic accordingly, or use a parallel sequence namespace (e.g., negative numbers or a separate field) so XState and agent events don't interfere with each other's ordering.
🟡 [QLT-2] [MEDIUM] Magic number 0 used without named constant
📁 apps/frontend/src/main/task-state-manager.ts:380
The value 0 is used as a sentinel/bypass value for the sequence guard, but this semantic meaning is only conveyed through the comment block. A named constant (e.g., SEQUENCE_BYPASS = 0) would make the intent explicit and ensure consistency if this pattern is used elsewhere.
Suggested fix:
Define a constant like `const SEQUENCE_NUMBER_BYPASS = 0;` at module level and use it here: `sequenceNumber: SEQUENCE_NUMBER_BYPASS`
🟡 [QLT-3] [MEDIUM] Hard-coded progress values 50 and 100 without constants or documentation
📁 apps/frontend/src/main/task-state-manager.ts:377
The phaseProgress and overallProgress values use magic numbers 50 and 100. While 100 for 'complete' is intuitive, 50 as the default for all non-complete phases is arbitrary and may not accurately represent progress. These should be named constants, and the choice of 50% as a fallback deserves documentation or a more nuanced mapping.
Suggested fix:
Extract named constants (e.g., `PHASE_IN_PROGRESS = 50`, `PHASE_COMPLETE = 100`) and consider whether different phases should report different progress percentages.
🔵 [QLT-4] [LOW] Test plan has unchecked manual test item
📁 apps/frontend/src/main/task-state-manager.ts:380
The PR description's test plan includes an unchecked manual test ([ ] Start a task → observe kanban board → card should update from 'planning' to 'coding'). Given that this is the exact scenario the fix targets, merging without completing this manual verification is risky, especially since the sequenceNumber: 0 approach has the edge case noted in QLT-1.
Suggested fix:
Complete the manual test before merging, or add an integration/e2e test that verifies the kanban card transitions through planning → coding → complete.
🟠 [DEEP-1] [HIGH] sequenceNumber: 0 allows out-of-order XState phase events to overwrite newer agent events
📁 apps/frontend/src/main/task-state-manager.ts:380
Setting sequenceNumber: 0 fixes the original problem where Date.now() poisoned the sequence counter, but it introduces a new race condition. If an agent has already emitted events with sequenceNumber >= 1, and then an XState transition fires (e.g., a delayed or re-entrant state change), the renderer will accept the XState event (seq 0 < currentSeq would be dropped by the guard... wait, actually seq 0 would be DROPPED by the guard incomingSeq < currentSeq). This means XState phase transitions that arrive AFTER any agent event will be silently discarded. For example, if the agent emits seq=1 (coding progress), and then XState transitions to 'complete', the completion event with seq=0 will be dropped, and the card will never show 'complete' from XState. The fix only works for the initial race (planning → coding) but breaks the terminal state transition (coding → complete) if the agent has already sent any sequenced events.
Suggested fix:
Use a dedicated mechanism for XState phase updates that bypasses the sequence guard entirely (e.g., a separate IPC channel, a 'force' flag on the payload, or use `sequenceNumber: Infinity` / `Number.MAX_SAFE_INTEGER` to guarantee acceptance). Alternatively, refactor the renderer to treat phase-level transitions differently from incremental progress updates, so they are never subject to the out-of-order guard.
🟡 [DEEP-2] [MEDIUM] Hardcoded progress values (50/100) create misleading UI state
📁 apps/frontend/src/main/task-state-manager.ts:378
The phaseProgress and overallProgress are hardcoded to 50 for non-complete phases and 100 for complete. When sequenceNumber is 0 and this event IS accepted (i.e., it arrives before any agent events), it will set progress to 50% immediately. If the agent subsequently sends its own progress events (e.g., 10%, 20%, 30%), the user could see progress jump from 50% back to 10%, creating a confusing UX. With the old Date.now() approach, agent events were dropped so this regression wasn't visible — but with seq=0, the agent events WILL flow through, causing a non-monotonic progress bar.
Suggested fix:
Set phaseProgress and overallProgress to 0 (or a small initial value like 1) for non-complete phases, so that subsequent agent progress events only move the bar forward.
🟡 [DEEP-3] [MEDIUM] Two independent sequencing domains (XState vs agent) sharing one sequence counter creates fundamental design fragility
📁 apps/frontend/src/main/task-state-manager.ts:380
The root cause described in the PR — two different event sources using incompatible sequencing schemes fed into one out-of-order guard — is only partially addressed. The fix changes one magic number to another magic number. Any future code that emits to TASK_EXECUTION_PROGRESS with a different sequencing assumption will re-introduce the same class of bug. There is no type-level or runtime enforcement that sequence numbers from different sources are compatible.
Suggested fix:
Introduce separate sequence counters per source (e.g., `xstateSeq` and `agentSeq`) in the renderer, or add a `source` field to the event payload so the renderer can maintain independent sequence tracking. Alternatively, use a monotonic counter shared across all emitters (e.g., an atomic incrementing counter in TaskStateManager).
🔵 [DEEP-4] [LOW] Manual test plan item is unchecked — no automated integration test for the fixed scenario
📁 apps/frontend/src/main/task-state-manager.ts:380
The PR description shows the critical test case ('Start a task → observe kanban board → card should update from planning to coding') is unchecked. The fix changes subtle sequencing behavior that unit tests alone cannot validate (as evidenced by all 3467 unit tests passing before AND after the fix). Without an integration or e2e test covering this scenario, the bug is likely to regress.
Suggested fix:
Add an integration test that simulates an XState phase transition followed by agent progress events, asserting that both are received by the renderer in order and that progress updates are not dropped.
This review was generated by Auto Claude.
…t regression Setting phaseProgress and overallProgress to 0 (instead of 50) when XState emits a phase-change event ensures the progress bar only moves forward. Previously, XState would set progress to 50%, then the agent would send its actual progress (e.g. 10%), causing the bar to visually regress from 50% → 10%. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
bot
added
size/S
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/frontend/src/main/task-state-manager.ts`:
- Around line 377-378: The progress calculation resets to 0 for error states
because phaseProgress and overallProgress use phase === 'complete' ? 100 : 0;
update the logic in the code that sets phaseProgress and overallProgress (the
lines that compute these values) to treat terminal failure as complete—either
change the condition to include 'failed' (e.g., check
['complete','failed'].includes(phase)) or replace with an explicit mapping for
terminal phases (e.g., map
'complete','done','pr_created','creating_pr','human_review','failed' to 100) so
error states show 100% instead of 0%.
- Around line 365-381: The renderer's sequence guard (the check using
incomingSeq and currentSeq in the task-store update path that merges
IPC_CHANNELS.TASK_EXECUTION_PROGRESS payloads) must treat sequenceNumber=0 as a
special sentinel so mid-task updates don't reset progress; update that guard
(the condition that currently reads incomingSeq > 0 && currentSeq > 0 &&
incomingSeq < currentSeq) to also skip when incomingSeq === 0 && currentSeq > 0
(or alternatively change the main-process sender safeSendToRenderer for
IPC_CHANNELS.TASK_EXECUTION_PROGRESS to use a distinct sentinel like -1 and
update the renderer's guard to handle that sentinel), ensuring variables
referenced (incomingSeq/currentSeq and the TASK_EXECUTION_PROGRESS handler that
merges phaseProgress/overallProgress) are adjusted accordingly.
| // Emit execution progress with the phase derived from XState. | ||
| // IMPORTANT: Do NOT use Date.now() as sequenceNumber here — doing so would | ||
| // permanently block all subsequent agent execution-progress events, because | ||
| // the agent uses small sequential integers (1, 2, 3…) that are always less | ||
| // than a timestamp. Instead emit with sequenceNumber=0 so the agent's own | ||
| // events can continue updating phaseProgress once the phase is established. | ||
| safeSendToRenderer( | ||
| this.getMainWindow, | ||
| IPC_CHANNELS.TASK_EXECUTION_PROGRESS, | ||
| taskId, | ||
| { | ||
| phase, | ||
| phaseProgress: phase === 'complete' ? 100 : 50, | ||
| overallProgress: phase === 'complete' ? 100 : 50, | ||
| phaseProgress: phase === 'complete' ? 100 : 0, | ||
| overallProgress: phase === 'complete' ? 100 : 0, | ||
| message: `State: ${xstateState}`, | ||
| sequenceNumber: Date.now() // Use timestamp as sequence to ensure it's newer | ||
| sequenceNumber: 0 | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "updateExecutionProgress|incomingSeq|currentSeq|sequenceNumber" --type ts --type tsx -C 3 apps/frontend/src/renderer/Repository: AndyMik90/Auto-Claude
Length of output: 92
🏁 Script executed:
rg -n "updateExecutionProgress|incomingSeq|currentSeq" apps/frontend/src/renderer/ -C 3Repository: AndyMik90/Auto-Claude
Length of output: 8389
🏁 Script executed:
sed -n '416,460p' apps/frontend/src/renderer/stores/task-store.tsRepository: AndyMik90/Auto-Claude
Length of output: 1875
Sequence guard doesn't filter sequenceNumber: 0 mid-task—progress bar will reset.
The renderer's sequence guard at line 435 of apps/frontend/src/renderer/stores/task-store.ts only drops updates when incomingSeq > 0 && currentSeq > 0 && incomingSeq < currentSeq. When the main process sends sequenceNumber: 0 (as this fix does), the condition incomingSeq > 0 is false, so the guard is bypassed entirely. The update proceeds to merge at line 451-454, applying phaseProgress: 0 to the task state.
This means every XState mid-task transition (e.g., planning → coding while the agent is running) will reset the progress bar to 0%, which is the exact regression the PR commit message claims to avoid. The sequence guard has no logic to treat sequenceNumber: 0 as a special sentinel that should be dropped when currentSeq > 0.
Either update the renderer's guard to explicitly drop sequenceNumber: 0 when currentSeq > 0:
if ((incomingSeq === 0 && currentSeq > 0) || (incomingSeq > 0 && currentSeq > 0 && incomingSeq < currentSeq)) {
return t; // Skip update
}Or use a distinct sentinel value (e.g., sequenceNumber: -1) with explicit renderer handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/main/task-state-manager.ts` around lines 365 - 381, The
renderer's sequence guard (the check using incomingSeq and currentSeq in the
task-store update path that merges IPC_CHANNELS.TASK_EXECUTION_PROGRESS
payloads) must treat sequenceNumber=0 as a special sentinel so mid-task updates
don't reset progress; update that guard (the condition that currently reads
incomingSeq > 0 && currentSeq > 0 && incomingSeq < currentSeq) to also skip when
incomingSeq === 0 && currentSeq > 0 (or alternatively change the main-process
sender safeSendToRenderer for IPC_CHANNELS.TASK_EXECUTION_PROGRESS to use a
distinct sentinel like -1 and update the renderer's guard to handle that
sentinel), ensuring variables referenced (incomingSeq/currentSeq and the
TASK_EXECUTION_PROGRESS handler that merges phaseProgress/overallProgress) are
adjusted accordingly.
| phaseProgress: phase === 'complete' ? 100 : 0, | ||
| overallProgress: phase === 'complete' ? 100 : 0, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -nP "XSTATE_TO_PHASE" --type ts -C 10Repository: AndyMik90/Auto-Claude
Length of output: 20638
Handle 'failed' phase state in progress calculation.
The phase === 'complete' ? 100 : 0 logic only recognizes 'complete' as a finished state. Terminal states map mostly correctly to 'complete' ('done', 'pr_created', 'creating_pr', 'human_review'), but the 'error' state maps to 'failed', causing the progress bar to reset to 0% when a task errors out. Either extend the condition to include 'failed' (e.g., ['complete', 'failed'].includes(phase) ? 100 : 0) or define explicit progress values for each terminal phase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/main/task-state-manager.ts` around lines 377 - 378, The
progress calculation resets to 0 for error states because phaseProgress and
overallProgress use phase === 'complete' ? 100 : 0; update the logic in the code
that sets phaseProgress and overallProgress (the lines that compute these
values) to treat terminal failure as complete—either change the condition to
include 'failed' (e.g., check ['complete','failed'].includes(phase)) or replace
with an explicit mapping for terminal phases (e.g., map
'complete','done','pr_created','creating_pr','human_review','failed' to 100) so
error states show 100% instead of 0%.
1 participant
Summary
emitPhaseFromStateintask-state-manager.tsusedsequenceNumber: Date.now()(~1.7 trillion) for XState phase transitions. The renderer'supdateExecutionProgresshas an out-of-order guard that drops events whereincomingSeq < currentSeq. Once the initial "planning" phase setcurrentSeqto a timestamp, ALL subsequent agent progress events (using small sequential integers: 1, 2, 3...) were permanently dropped, freezing the card on "planning"sequenceNumber: Date.now()tosequenceNumber: 0so XState updates bypass the sequence guard and don't poison the sequence counter for subsequent agent eventsFixes #1885
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit