fix(server): workflow phases no longer vanish from the Agents panel - #8952
chomamateusz wants to merge 2 commits into
Conversation
Workflow phase chips (pending later phases) flickered out and back in the Agents panel during a run. The coordinator's phase list only rides on material workflow_progress ticks; a plain progress tick (token/tool update, no phases) upserting the shared task-progress activity id blanked the phase list until the next material tick. Emit phases under their own stable task-phases id so a phaseless tick can never overwrite them. The row is metadata-only (usageSnapshot) so the client fold does not read it as a status transition.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe runtime ingestion logic now emits a dedicated activity snapshot when ChangesRuntime ingestion
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR separates workflow phase snapshots from ordinary progress updates so pending phases remain visible in the Agents panel; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description includes the required What Changed, Why, UI Changes, and Checklist sections. It explains the root cause and implementation, provides before-and-after media for the UI change, and marks the checklist items complete.
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment Warning |
| createdAt: event.createdAt, | ||
| tone: "info" as const, | ||
| kind: "task.progress" as const, | ||
| summary: "Workflow phases updated", |
There was a problem hiding this comment.
🟡 Medium Layers/ProviderRuntimeIngestion.ts:660
The phase snapshot overwrites the coordinator's real current activity with "Workflow phases updated" because it is emitted as a task.progress row with a non-empty summary, and the client fold processes that summary even when usageSnapshot is set. Use an empty summary for this phase-only row so it updates phases without replacing the displayed activity.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts around line 660:
The phase snapshot overwrites the coordinator's real current activity with `"Workflow phases updated"` because it is emitted as a `task.progress` row with a non-empty summary, and the client fold processes that summary even when `usageSnapshot` is set. Use an empty summary for this phase-only row so it updates phases without replacing the displayed activity.
There was a problem hiding this comment.
I don't think this can happen — foldSubagentActivities only reads payload.summary (
summary key; the string only exists on the activity row itself. It mirrors the existing task-usage: row (
), which doesn't surface as displayed activity either.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a narrow server-side bug fix that preserves workflow phases in a stable activity snapshot and adds targeted coverage, without schema, security, deployment, or default changes. An unresolved Medium finding does identify that the snapshot summary can overwrite the coordinator’s displayed activity text, which remains a merge blocker under the repository’s configured threshold. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
|
Thanks for the PR. We're not taking changes to the orchestration and provider layers right now: that part of the server is being rewritten for V2, and merging into the current code would either conflict with or be thrown away by that work. Closing for now. If this is still an issue once V2 lands, please reopen (or open a fresh PR against the new code) and we'll take a proper look. |
What Changed
Workflow phases now live in their own activity row with a stable id (
task-phases:${threadId}:${taskId}) instead of riding on the coordinator'stask-progress:row. The row is written only when an event actually carries phases, so nothing can overwrite it with an empty list. It is markedusageSnapshot, which keeps the client fold from reading it as a status change.Why
Pending phase chips flicker out and back in the Agents panel during a run.
task.progressupserts the coordinator under one stabletask-progress:id, so a later tick without phases overwrites the row that had them; the fold then keeps only phases with live members and drops the pending ones until the next tick. A dedicatedtask-phases:row can't be blanked by a phaseless tick.Different from #8720 / #8749, which fix the between-phases agent count in
client-runtime— this is the phase list dropped at server ingestion during a phase.UI Changes
Before — the pending
Sequentialchip and its greyed-out section blink in and out during the run (sped up ~5x; the appearances are held slightly longer so they're visible):t3-phases-before.mp4
After — the pending phase stays visible for the whole run:
t3-phases-after.mp4
Stills from the same before run, seconds apart — the greyed-out
Sequentialsection is there, then gone. After the fix it stays put:Checklist
Model: Claude Fable 5
Harness: Claude Code in T3 Code
Note
Low Risk
Scoped change to
task.progressactivity projection and tests; no auth, persistence schema, or client code changes in this diff.Overview
Workflow phase chips in the Agents panel stop flickering because server ingestion no longer stores phases on the same upserted
task-progress:activity that plain progress ticks overwrite.When a
task.progressevent includes a non-emptyphasesarray, ingestion now also emits a separate activity with idtask-phases:{threadId}:{taskId}, mirroring the existing split betweentask-progress:andtask-usage:. That row is only created on ticks that carry phases, is markedusageSnapshot: trueso the client fold does not treat it as a status change, and includes phase metadata via the same linkage fields as other task activities. Phaseless ticks still updatetask-progress:only, so they cannot blank the last known phase list.A regression test asserts phases survive on the dedicated row and that a plain progress tick does not emit or replace
task-phases:.Reviewed by Cursor Bugbot for commit fb99513. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Emit dedicated
task-phasesactivity inruntimeEventToActivitiesfortask.progresseventstask.progressbranch now detectsevent.payload.phasesviahasPhasesand emits an additional activity with idtask-phases:<threadId>:<taskId>, toneinfo, kindtask.progress, andusageSnapshot: true.runtimeEventToActivitiesnow produces an extra activity for anytask.progressevent with a non-emptyphasesarray; downstream consumers of thetask-phases:<threadId>:<taskId>id format should be checked.📊 Macroscope summarized d383942. 1 file reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues
Summary by CodeRabbit
New Features
Bug Fixes