Sidebar v2: server-backed settled lifecycle (settledAt)#4243
Conversation
thread.settle / thread.unsettle commands, thread.settled/unsettled events, settled_override + settled_at projection columns (migration 033), and activity auto-unsettle in the decider: a user message, a session coming alive, or a new approval/user-input request emits thread.unsettled(reason: activity) ahead of the triggering event. Settled threads stay in the live shell stream (unlike archived ones): settledOverride/settledAt ride the existing thread shell schema, so readable settled history requires no separate snapshot path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
settle/unsettle now dispatch thread.settle / thread.unsettle instead of riding archive/unarchive, and effectiveSettled keys on the projected settledOverride/settledAt. Because settled threads stay in the live shell stream, the archive-era workarounds are deleted: - settledHolds optimistic-hold machinery (web + mobile) - archived-snapshot merging into the v2 lists - unarchive-on-open when activating a settled row — opening is plain navigation now, and history stays readable without un-settling - the auto-settled un-settle special case: reason 'user' pins any settled row active until real activity clears the pin Archive regains its original meaning (hidden from lists) and is no longer offered as settle's alias anywhere in v2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Needs human review This PR introduces a new server-backed thread settlement lifecycle with database migration, new commands/events, and significant client-side changes. Multiple unresolved review comments identify potential data integrity issues (updatedAt regression) and edge-case bugs (queued turn handling). These substantive concerns warrant careful human review. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/server/src/orchestration/decider.ts`:
- Around line 330-358: Update the "thread.settle" branch in the decider so
re-settling preserves the original settledAt while setting updatedAt to a
non-decreasing timestamp, such as the later of the current persisted
thread.updatedAt and settledAt. Update decider.settled.test.ts to assert that an
advanced existing updatedAt is retained rather than regressed.
In `@apps/server/src/persistence/Layers/ProjectionRepositories.test.ts`:
- Around line 94-95: Expand the settlement lifecycle coverage: in
apps/server/src/persistence/Layers/ProjectionRepositories.test.ts:94-95, persist
“settled” with an ISO timestamp, read it back, then upsert “active” with a null
timestamp and assert both transitions; in
apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts:311-312,
add a settled projection scenario and assert non-null fields in the full
snapshot; and at :423-424, assert the same lifecycle values in the shell
snapshot. Run the focused tests for both files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3e90a8ce-62a8-472b-93cf-cbf3fcb22ff0
📒 Files selected for processing (51)
apps/mobile/src/features/archive/archivedThreadList.test.tsapps/mobile/src/features/home/HomeRouteScreen.tsxapps/mobile/src/features/home/HomeScreen.tsxapps/mobile/src/features/home/homeListItems.test.tsapps/mobile/src/features/home/homeThreadList.test.tsapps/mobile/src/features/home/useThreadListActions.tsapps/mobile/src/features/threads/thread-list-v2-items.tsxapps/mobile/src/features/threads/threadListV2.test.tsapps/mobile/src/lib/repositoryGroups.test.tsapps/mobile/src/lib/threadActivity.test.tsapps/mobile/src/state/use-thread-selection.tsapps/server/src/orchestration/Layers/OrchestrationEngine.test.tsapps/server/src/orchestration/Layers/ProjectionPipeline.tsapps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.tsapps/server/src/orchestration/Layers/ProjectionSnapshotQuery.tsapps/server/src/orchestration/Schemas.tsapps/server/src/orchestration/commandInvariants.test.tsapps/server/src/orchestration/decider.settled.test.tsapps/server/src/orchestration/decider.tsapps/server/src/orchestration/projector.settled.test.tsapps/server/src/orchestration/projector.test.tsapps/server/src/orchestration/projector.tsapps/server/src/persistence/Layers/ProjectionRepositories.test.tsapps/server/src/persistence/Layers/ProjectionThreads.tsapps/server/src/persistence/Migrations.tsapps/server/src/persistence/Migrations/033_ProjectionThreadsSettled.tsapps/server/src/persistence/Services/ProjectionThreads.tsapps/server/src/provider/Layers/ProviderSessionReaper.test.tsapps/server/src/relay/AgentAwarenessRelay.test.tsapps/server/src/server.test.tsapps/web/src/components/ChatView.logic.test.tsapps/web/src/components/ChatView.logic.tsapps/web/src/components/CommandPalette.logic.test.tsapps/web/src/components/Sidebar.logic.test.tsapps/web/src/components/SidebarV2.tsxapps/web/src/hooks/useThreadActions.tsapps/web/src/lib/threadSort.test.tsapps/web/src/worktreeCleanup.test.tspackages/client-runtime/src/operations/commands.test.tspackages/client-runtime/src/operations/commands.tspackages/client-runtime/src/state/entities.test.tspackages/client-runtime/src/state/shellReducer.test.tspackages/client-runtime/src/state/threadCommands.tspackages/client-runtime/src/state/threadDetail.tspackages/client-runtime/src/state/threadReducer.test.tspackages/client-runtime/src/state/threadReducer.tspackages/client-runtime/src/state/threadSettled.test.tspackages/client-runtime/src/state/threadSettled.tspackages/client-runtime/src/state/threads-sync.test.tspackages/contracts/src/orchestration.test.tspackages/contracts/src/orchestration.ts
| case "thread.settle": { | ||
| const thread = yield* requireThreadNotArchived({ | ||
| readModel, | ||
| command, | ||
| threadId: command.threadId, | ||
| }); | ||
| const occurredAt = yield* nowIso; | ||
| // Settling an already-settled thread re-emits with the original | ||
| // settledAt: the engine rejects zero-event commands, and bulk-settle / | ||
| // double-click must stay silent no-ops rather than surface errors. | ||
| const settledAt = | ||
| thread.settledOverride === "settled" && thread.settledAt !== null | ||
| ? thread.settledAt | ||
| : occurredAt; | ||
| return { | ||
| ...(yield* withEventBase({ | ||
| aggregateKind: "thread", | ||
| aggregateId: command.threadId, | ||
| occurredAt, | ||
| commandId: command.commandId, | ||
| })), | ||
| type: "thread.settled", | ||
| payload: { | ||
| threadId: command.threadId, | ||
| settledAt, | ||
| updatedAt: settledAt, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Idempotent re-settle can move thread.updatedAt backward.
updatedAt: settledAt reuses the original settled timestamp on re-emit. If the thread's persisted updatedAt has since advanced past settledAt for any unrelated reason, this event regresses it — thread.updatedAt is a general last-modified watermark (used for sorting/snapshot freshness in ProjectionSnapshotQuery), and it shouldn't move backward. The corresponding test (decider.settled.test.ts lines 96-102) currently locks in this exact regression and would need updating alongside the fix.
🐛 Proposed fix
const settledAt =
thread.settledOverride === "settled" && thread.settledAt !== null
? thread.settledAt
: occurredAt;
return {
...
type: "thread.settled",
payload: {
threadId: command.threadId,
settledAt,
- updatedAt: settledAt,
+ updatedAt: occurredAt,
},
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "thread.settle": { | |
| const thread = yield* requireThreadNotArchived({ | |
| readModel, | |
| command, | |
| threadId: command.threadId, | |
| }); | |
| const occurredAt = yield* nowIso; | |
| // Settling an already-settled thread re-emits with the original | |
| // settledAt: the engine rejects zero-event commands, and bulk-settle / | |
| // double-click must stay silent no-ops rather than surface errors. | |
| const settledAt = | |
| thread.settledOverride === "settled" && thread.settledAt !== null | |
| ? thread.settledAt | |
| : occurredAt; | |
| return { | |
| ...(yield* withEventBase({ | |
| aggregateKind: "thread", | |
| aggregateId: command.threadId, | |
| occurredAt, | |
| commandId: command.commandId, | |
| })), | |
| type: "thread.settled", | |
| payload: { | |
| threadId: command.threadId, | |
| settledAt, | |
| updatedAt: settledAt, | |
| }, | |
| }; | |
| } | |
| case "thread.settle": { | |
| const thread = yield* requireThreadNotArchived({ | |
| readModel, | |
| command, | |
| threadId: command.threadId, | |
| }); | |
| const occurredAt = yield* nowIso; | |
| // Settling an already-settled thread re-emits with the original | |
| // settledAt: the engine rejects zero-event commands, and bulk-settle / | |
| // double-click must stay silent no-ops rather than surface errors. | |
| const settledAt = | |
| thread.settledOverride === "settled" && thread.settledAt !== null | |
| ? thread.settledAt | |
| : occurredAt; | |
| return { | |
| ...(yield* withEventBase({ | |
| aggregateKind: "thread", | |
| aggregateId: command.threadId, | |
| occurredAt, | |
| commandId: command.commandId, | |
| })), | |
| type: "thread.settled", | |
| payload: { | |
| threadId: command.threadId, | |
| settledAt, | |
| updatedAt: occurredAt, | |
| }, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/server/src/orchestration/decider.ts` around lines 330 - 358, Update the
"thread.settle" branch in the decider so re-settling preserves the original
settledAt while setting updatedAt to a non-decreasing timestamp, such as the
later of the current persisted thread.updatedAt and settledAt. Update
decider.settled.test.ts to assert that an advanced existing updatedAt is
retained rather than regressed.
| settledOverride: null, | ||
| settledAt: null, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Exercise non-null settlement state end-to-end.
These assertions only cover null, so a lost alias, write binding, or unset transition can ship unnoticed.
apps/server/src/persistence/Layers/ProjectionRepositories.test.ts#L94-L95: persist"settled"with an ISO timestamp, read it back, then upsert"active"withnulltimestamp and assert both transitions.apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts#L311-L312: add a settled projection scenario and assert non-null fields in the full snapshot.apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts#L423-L424: assert the same lifecycle values in the shell snapshot.
Run the focused server tests with vp test run apps/server/src/persistence/Layers/ProjectionRepositories.test.ts apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts. As per coding guidelines, backend changes must include and run focused tests for the changed behavior.
📍 Affects 2 files
apps/server/src/persistence/Layers/ProjectionRepositories.test.ts#L94-L95(this comment)apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts#L311-L312apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts#L423-L424
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/server/src/persistence/Layers/ProjectionRepositories.test.ts` around
lines 94 - 95, Expand the settlement lifecycle coverage: in
apps/server/src/persistence/Layers/ProjectionRepositories.test.ts:94-95, persist
“settled” with an ISO timestamp, read it back, then upsert “active” with a null
timestamp and assert both transitions; in
apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts:311-312,
add a settled projection scenario and assert non-null fields in the full
snapshot; and at :423-424, assert the same lifecycle values in the shell
snapshot. Run the focused tests for both files.
Source: Coding guidelines
- Real activity now clears ANY settled override, not just 'settled': the keep-active pin resets to neutral on a user message, live session, or approval/user-input request, so auto-settle applies again once that work goes stale. The pin exists to suppress AUTO-settle, not to survive real work. (The three tests codifying the old behavior are reversed.) - New threadSettlement capability on the environment descriptor (absent = unsupported): web and mobile refuse to dispatch settle/unsettle to a pre-settlement server with a clear message instead of a raw protocol rejection. - Idempotent re-settle keeps the original settledAt but no longer rewinds updatedAt to it (sorting and relative-time labels key on updatedAt). - Server-side twin of canSettle's session check: thread.settle is rejected while a session is starting/running. Pending approval/user-input blocking stays client-side, backed by effectiveSettled rendering blocked threads active and the activity gate auto-un-settling on new requests. - ThreadUnsettleCommand reason narrowed to 'user': activity resets are decided server-side and cannot be forged by a client. - Persistence round-trip test for non-null settled values (upsert → getById across both override states); stale settle-is-archive comments removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rage The CI break: 'decodes thread settle and unsettle commands' still decoded reason 'activity' after the command schema was narrowed to 'user'. The test now decodes 'user' and asserts 'activity' is rejected (server-owned, event-only). Also per review: snapshot-query coverage proving a settled thread stays in the LIVE shell snapshot with non-null settledOverride/ settledAt through the row aliases, and appears in the command read model. (The repository round-trip for both override states landed in the previous commit; cursor's other two findings — active-pin clearing and updatedAt rewind — were fixed there too and are stale against this head.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Queued turn starts are unsettleable: new hasQueuedTurnStart (client) detects a user message no turn has picked up (newest user message strictly newer than every latestTurn timestamp; a failed session start clears the block); canSettle and effectiveSettled refuse it, and the decider enforces the same invariant server-side, so just-requested work can be neither settled nor auto-settled by a closed PR. - Capability-aware classification: threads on environments without threadSettlement (old server, or descriptor not yet loaded) never partition into the settled tail on web or mobile — a tail with no working un-settle/pin affordances was strandable. Unknown is treated as unsupported. - Post-settle navigation validates the CURRENT route before firing: a navigation made while the settle was in flight wins. - Scope flips clear the thread selection, and the bulk context menu derives one exact actionable set (rendered rows only) so labels count exactly what actions will touch. - Settle/unsettle re-emissions are projected no-ops: duplicates keep the existing updatedAt instead of churning ordering. - Web settle-blocked failures use a settlement-specific error matching mobile copy instead of ThreadArchiveBlockedError's archive wording. - New coverage: queued-turn tests (client + decider), settled event round-trip through the DB ProjectionPipeline, capability-gated partition fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 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 04c0791. Configure here.
| return [turn.requestedAt, turn.startedAt, turn.completedAt].every( | ||
| (candidate) => candidate == null || Date.parse(candidate) < messageAt, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Queued settle lock can stick
Medium Severity
hasQueuedTurnStart only clears on session status error. If a user message lands and latestTurn never appears while session stays null or another non-live status (stopped/idle/ready), settle and auto-settle stay blocked on both client and server with no recovery path short of delete.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 04c0791. Configure here.
| // the actions will touch. | ||
| const threadKeys = [...useThreadSelectionStore.getState().selectedThreadKeys].filter( | ||
| (threadKey) => threadByKeyRef.current.has(threadKey), | ||
| ); |
There was a problem hiding this comment.
Ghost multi-select after delete
Low Severity
Bulk delete now filters actions to keys present in threadByKeyRef (visible rows only), then calls removeFromSelection with that filtered list. Selected rows that dropped out of the rendered set remain selected as ghosts, unlike settle/mark-unread which clear the whole selection.
Reviewed by Cursor Bugbot for commit 04c0791. Configure here.
| const hasQueuedTurnStart = | ||
| thread.session?.status !== "error" && | ||
| Number.isFinite(latestUserMessageAtMs) && | ||
| latestUserMessageAtMs > latestTurnAtMs; |
There was a problem hiding this comment.
Settle guard misses queued turns
Medium Severity
The new thread.settle queued-turn check derives activity from thread.messages, but getCommandReadModel always hydrates threads with messages: []. After a server restart (or any cold command-model load) while a turn is still queued, the guard never sees the user message, so settle can succeed despite in-flight work. The client blocks via latestUserMessageAt; this server twin does not.
Reviewed by Cursor Bugbot for commit 04c0791. Configure here.
|
Folded into #4026 via fast-forward — the base branch now contains all five commits from this PR, so GitHub auto-marked it merged. Review continues on #4026, which is now the single PR for the whole beta (client UI + server settled lifecycle, shipped together so the intermediate settle-as-archive state never deploys). |


What
Stacked on #4026 (base branch:
t3code/new-sidebar-client-only) — do not merge before it.Replaces the client-only "settle rides archive" model with a real server-backed settled lifecycle, ported from the #3960 spike:
thread.settle/thread.unsettlecommands,thread.settled/thread.unsettledevents,settled_override+settled_atcolumns onprojection_threads(migration 033). The decider auto-un-settles on real activity: a user message, a session coming alive, or a new approval/user-input request emitsthread.unsettled(reason: "activity")ahead of the triggering event.effectiveSettledkeys onsettledOverride/settledAt.unsettle(reason: "user")pins a thread active, suppressing auto-settle until real activity.Why (what this fixes structurally)
Settled threads now stay in the live shell stream — settled ≠ archived. That one property deletes the whole workaround layer the client-only model needed:
settledHoldsoptimistic-hold machinery (web + mobile)Safety
sidebarV2Enabled/threadListV2Enabled, default off.PRAGMA-guardedALTER TABLE), no backfill, no data rewritten.Review focus
apps/server/src/orchestration/decider.ts— settle/unsettle command handling + the three activity-un-settle injection points (message send, session set, activity append). Idempotency-by-re-emission is documented inline.apps/server/src/orchestration/projector.ts+ProjectionPipeline.ts—reason: "user"→ override"active"(pin),reason: "activity"→ overridenull(neutral).packages/client-runtime/src/state/threadSettled.ts— the settled resolution order: blockers → override → PR state → inactivity.🤖 Generated with Claude Code
Note
High Risk
Touches orchestration commands/events, projections, and sidebar/home navigation across web and mobile; incorrect activity-unsettle or settle guards could hide live work or strand threads on mixed server versions.
Overview
Settle is no longer archive. Clients and server gain a first-class settled lifecycle:
thread.settle/thread.unsettle(user pin viareason: "user"),thread.settled/thread.unsettledevents, and nullablesettled_override/settled_atonprojection_threads(migration 033). The environment advertisescapabilities.threadSettlement; older servers are gated so settle/unsettle are not sent.Settled threads stay in the live shell stream (
archivedAtstill means hidden from lists).effectiveSettledandcanSettlekey onsettledOverride/settledAt(plus PR merge/close and inactivity), with new queued turn start blocking so a just-sent message cannot be settled. The decider rejects settle on live sessions or queued work, idempotently re-emits settle/unsettle, and prependsthread.unsettled(reason: "activity")before turn starts, live session sets, and approval/user-input activities when any override is set.Web and mobile v2 lists drop the client-only layer: no archived-snapshot merge, no
settledHolds, no unarchive-on-open—navigation to a settled thread is direct; messaging/session work un-settles server-side. List partitioning uses live unarchived shells and capability-gated settlement; archived-snapshot refresh is limited to archive/delete paths.Reviewed by Cursor Bugbot for commit 04c0791. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add server-backed thread settle/unsettle lifecycle with
settledAttrackingthread.settleandthread.unsettlecommands end-to-end: new contract schemas, server decider logic, projection pipeline persistence, and client-side dispatch viasettleThread/unsettleThreadinclient-runtime.settledOverrideandsettledAtfields toOrchestrationThread/OrchestrationThreadShelland theprojection_threadstable (migration 033), replacing the previousarchivedAt-based settlement classification.effectiveSettlednow prioritizes an explicitsettledOverridevalue ('settled'/'active') over auto-settle heuristics;canSettlegains a new blocker for queued-but-not-yet-adopted turn starts viahasQueuedTurnStart.thread.unsettled(reason'activity') when real activity arrives on a settled thread (new user message, session starting/running, or approval/user-input request).capabilities.threadSettlement: true, exclude archived threads from the live list, and navigate directly to settled threads without pre-emptively unsettling.settleThread/unsettleThreadactions now dispatch explicit server commands instead ofarchive/unarchive; optimisticsettledHoldsand archived snapshot merging are removed from both web and mobile.Macroscope summarized 04c0791.
Summary by CodeRabbit
New Features
Bug Fixes