feat(server): server-stamped task data shapes (stacked on #5219) - #5316
Conversation
…ntKind
Three write-time shape changes that let read-time complexity retire, all
inside payload_json / existing activity ids (still zero migrations):
- task.progress and agent-owned tool.progress persist under stable ids
(task-progress:<taskId>) so each tick REPLACES the last via the
existing activity_id upsert instead of appending. One progress row per
task means a large fleet can no longer evict its own start/terminal
rows out of the 500-row retention window (known fleet-reload gap).
- Ingestion stamps agentKind ("agent" | "background") into every task
linkage payload via classifyTaskAgentKind in contracts. Clients trust
the stamp outright; the fold's taskType denylist and legacy-marker
sniffing remain only as a fallback for pre-stamp rows.
- MONITOR_TASK_TYPES / INERT_TASK_TYPES move to contracts as the single
canonical copies; ThreadBackgroundLiveness consumes them instead of
keeping its own drift-prone sets.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| // progress row per task instead of thousands, so a large fleet's | ||
| // ticks can no longer evict its own start/terminal rows out of | ||
| // the 500-row retention window. | ||
| id: EventId.make(`task-progress:${event.payload.taskId}`), |
There was a problem hiding this comment.
🟠 High Layers/ProviderRuntimeIngestion.ts:574
task-progress:${event.payload.taskId} and tool-progress:${event.payload.taskId} are not scoped by thread, so the same task ID in two threads produces the same activity ID. Because activity_id is the global primary key and the upsert updates thread_id, a progress event in the second thread overwrites the first thread's row, corrupting both threads' persisted activity views. Include event.threadId in these IDs so they are globally unique.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts around line 574:
`task-progress:${event.payload.taskId}` and `tool-progress:${event.payload.taskId}` are not scoped by thread, so the same task ID in two threads produces the same activity ID. Because `activity_id` is the global primary key and the upsert updates `thread_id`, a progress event in the second thread overwrites the first thread's row, corrupting both threads' persisted activity views. Include `event.threadId` in these IDs so they are globally unique.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 498881e. Configure here.
| taskType: typeof payload.taskType === "string" ? payload.taskType : undefined, | ||
| agentId: typeof payload.agentId === "string" ? payload.agentId : undefined, | ||
| }), | ||
| }; |
There was a problem hiding this comment.
Partial rows flip agentKind stamp
Medium Severity
taskLinkageActivityFields always stamps agentKind from the current event only via classifyTaskAgentKind, which treats a missing taskType as agent. Progress and terminal rows often omit linkage that was present on start, so a shell, monitor, or plan can be stamped agent on later rows. Clients trust that stamp outright, so those tasks can enter the Agents roster and work-log spawn CTAs even when an earlier background start row exists or was retained.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 498881e. Configure here.
| // progress row per task instead of thousands, so a large fleet's | ||
| // ticks can no longer evict its own start/terminal rows out of | ||
| // the 500-row retention window. | ||
| id: EventId.make(`task-progress:${event.payload.taskId}`), |
There was a problem hiding this comment.
Stable progress IDs collide globally
High Severity
task.progress and agent-owned tool.progress now persist under task-progress:${taskId} / tool-progress:${taskId}, but projection_thread_activities.activity_id is a global primary key whose upsert also rewrites thread_id. taskId is only session-scoped, so two threads that share a task id fight over one row: progress moves or vanishes across threads on snapshot reload.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 498881e. Configure here.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Multiple unresolved review comments identify high-severity bugs: progress event IDs lack thread scoping (risking cross-thread data corruption), partial rows may receive incorrect agentKind stamps, and legacy pre-stamp rows are broken. These data integrity concerns require human review. You can customize Macroscope's approvability policy. Learn more. |
The agentKind stamp is now the ONLY classifier. isBackgroundTaskActivity collapses to one line (agentKind !== "agent"); the taskType denylist, legacy-marker sniffing, and agent-owned special-casing all delete from the client fold. Rows without a stamp — legacy threads, pre-stamp servers — simply don't join the roster, which is their pre-upgrade behavior. Sticky per-taskId membership still routes stampless later rows (defensive) to an agent created by a stamped one. Test fixtures stamp via the same classifyTaskAgentKind used by ingestion, so they model post-ingestion rows faithfully. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| } | ||
|
|
||
| /** True when this activity's payload describes a non-agent background task. */ | ||
| export function isBackgroundTaskActivity(payload: Record<string, unknown>): boolean { |
There was a problem hiding this comment.
🟡 Medium state/subagentRuntime.ts:117
isBackgroundTaskActivity returns true for every payload without agentKind, so persisted agent rows from pre-stamp servers — e.g. a task.started with taskType: "local_agent" or a workflow member carrying parentAgentId — fail the background guards in foldSubagentActivities and vanish from the Agents roster on reload. agentKind is optional in the persisted contract, so unstamped rows should fall back to the legacy task-type/marker classifier instead of being unconditionally treated as background.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/client-runtime/src/state/subagentRuntime.ts around line 117:
`isBackgroundTaskActivity` returns `true` for every payload without `agentKind`, so persisted agent rows from pre-stamp servers — e.g. a `task.started` with `taskType: "local_agent"` or a workflow member carrying `parentAgentId` — fail the background guards in `foldSubagentActivities` and vanish from the Agents roster on reload. `agentKind` is optional in the persisted contract, so unstamped rows should fall back to the legacy task-type/marker classifier instead of being unconditionally treated as background.
5d68958
into
t3code/native-subagent-observability


Note
Stacked on #5219 — merges into
t3code/native-subagent-observability, not main.Follow-up to the subagent observability PR: three write-time data-shape changes that let read-time complexity retire. Still zero migrations — everything rides
payload_jsonand the existingactivity_idupsert.Problem
The client fold compensates for append-only lifecycle rows at several points:
task.startedrows out of the 500-row retention window — degrading the roster on full reload.Solution
task.progress/ agent-ownedtool.progresspersist under stable ids (task-progress:<taskId>), so each tick replaces the last through the existing upsert + replace-by-id apply in the projector and client reducer. One progress row per task; retention pressure from progress disappears.agentKindstamp. Ingestion stamps"agent" | "background"into every task linkage payload viaclassifyTaskAgentKind(new in contracts). Clients trust the stamp; the fold's denylist and marker sniffing remain only as a fallback for pre-stamp rows.MONITOR_TASK_TYPES/INERT_TASK_TYPESmove to contracts;ThreadBackgroundLivenessconsumes them instead of keeping its own copies.Tests: classify unit tests in contracts, stamp-trust fold tests, ingestion suites updated for the stable ids (216 orchestration tests green).
Built by Claude Fable 5 via Claude Code.
Note
Medium Risk
Changes activity identity and upsert semantics for progress rows, plus agent roster classification that affects UI and liveness; legacy unstamped rows rely on fallback behavior.
Overview
Progress activities now upsert instead of appending:
task.progressand agent-ownedtool.progresspersist under stable ids (task-progress:<taskId>,tool-progress:<taskId>), so each tick replaces one row and large fleets no longer flood the 500-row activity retention window.Agent vs background classification is centralized in contracts via
classifyTaskAgentKind, sharedMONITOR_TASK_TYPES/INERT_TASK_TYPES, and anagentKindstamp on every task linkage payload at ingestion. The client fold trustsagentKind === "agent"and drops the denylist and pipeline-marker heuristics; rows without a stamp keep legacy work-log behavior.ThreadBackgroundLivenessimports the same contract sets instead of local copies.Tests and fixtures were updated for stable progress ids and stamped payloads.
Reviewed by Cursor Bugbot for commit ea06fbf. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Stamp server-classified
agentKindonto task activity payloads and stabilize progress activity IDsclassifyTaskAgentKindtopackages/contracts/src/providerRuntime.tsto classify tasks as'agent'or'background'using a shared denylist, replacing local copies in server and client.apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.tsnow stampsagentKindonto persistedtask.*activity payloads, making rows self-describing for downstream consumers.task.progressandtool.progressactivities are now upserted under stable per-task IDs (task-progress:{taskId},tool-progress:{taskId}) instead of per-event IDs, keeping only the latest heartbeat row and preventing progress storms from evicting start/terminal rows.subagentRuntime.isBackgroundTaskActivityinpackages/client-runtime/src/state/subagentRuntime.tsnow readspayload.agentKinddirectly instead of applying a denylist heuristic.agentKindwill be excluded from the agent roster.Macroscope summarized ea06fbf.