Work sidebar: settled thread lifecycle, two-tier attention, and the agent status surface (ADE-125)#886
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughChangesSession lifecycle integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/main/services/pty/ptyService.ts (1)
4348-4384: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear attention on every session-input write path.
write()clearsattentionRequestedand callssessionService.clearAttentionRequest, butwriteBySessionId()(used bysendToSession) andwriteTerminal()do not. If a tracked CLI session is marked as needing attention, input sent through either path leaves the persistedattention_requested_at/attention_messagerow in place.🤖 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/desktop/src/main/services/pty/ptyService.ts` around lines 4348 - 4384, Update writeBySessionId() and writeTerminal() to mirror write()’s attention-clearing behavior: when the target entry has attentionRequested, reset it and call sessionService.clearAttentionRequest(entry.sessionId) before writing input. Preserve the existing write behavior and apply this consistently to both alternate session-input paths.
🧹 Nitpick comments (2)
apps/ios/ADE/Views/Work/WorkRootComponents.swift (1)
1153-1176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
canonicalStaterecomputes whatrenderSignaturealready derived.
WorkSessionRowRenderSignature.init(872-873) already computesworkCanonicalSessionState(session:summary:)and storescanonicalPhase.canonicalState/capsuleBadge/isSettledhere independently call the same function again for the actual render, rather than reusing the signature's already-computed value. Not a functional bug today, but it's duplicated logic that could silently drift (e.g. if the signature'snowcapture and the render-timenowcapture ever need to differ intentionally, or if one call site is updated without the other).♻️ Suggested consolidation
Store the computed
CanonicalSessionState/badge onWorkSessionRowRenderSignatureonce, and haveWorkSessionRowread fromrenderSignatureinstead of recomputing:private struct WorkSessionRowRenderSignature: Equatable { ... let canonicalPhase: CanonicalSessionPhase + let canonicalBadge: SessionBadge? ... init(...) { ... let canonical = workCanonicalSessionState(session: session, summary: chatSummary) self.canonicalPhase = canonical.phase + self.canonicalBadge = canonical.badge ... } }- var capsuleBadge: SessionBadge? { - canonicalState.badge - } - - var canonicalState: CanonicalSessionState { - workCanonicalSessionState(session: session, summary: chatSummary) - } - - var isSettled: Bool { - canonicalState.phase == .settled - } + var capsuleBadge: SessionBadge? { renderSignature.canonicalBadge } + var isSettled: Bool { renderSignature.canonicalPhase == .settled }🤖 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/ios/ADE/Views/Work/WorkRootComponents.swift` around lines 1153 - 1176, Update WorkSessionRowRenderSignature to store the computed CanonicalSessionState and its badge once during initialization, then change WorkSessionRow’s canonicalState, capsuleBadge, and isSettled accessors to reuse renderSignature’s stored values instead of calling workCanonicalSessionState again. Preserve the existing rendering behavior and rowPreviewSource logic.apps/ade-cli/src/services/push/pushPublisherService.test.ts (1)
826-855: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an approval-to-question regression case.
This test starts with an empty run, so it cannot catch stale
itemIdor queued approval alerts. Seed a pending approval request first, then invokehandleSessionAttentionRequestedand assert that only the question alert remains and the Live Activity run has no approval item.🤖 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/ade-cli/src/services/push/pushPublisherService.test.ts` around lines 826 - 855, Update the test “publishes explicit CLI attention requests through the gated question path” to seed a pending approval request before calling handleSessionAttentionRequested. After advancing timers, assert that only the question notification remains and the Live Activity run contains no approval item, while preserving the existing question payload assertions.
🤖 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/ade-cli/src/services/push/pushPublisherService.ts`:
- Around line 1254-1259: When handling a new question in the session attention
flow, update the waiting_for_input transition in pushPublisherService.ts to
clear run.itemId, remove the pending approval alert, and clear its dedupe
fingerprint before enqueueing the question. Add a regression test in
pushPublisherService.test.ts covering an existing approval request followed by a
question request.
In `@apps/ade-cli/src/tuiClient/components/Drawer.tsx`:
- Around line 79-87: Reorder the lifecycle checks so failed status takes
precedence over settled status: update chatStatusDot in
apps/ade-cli/src/tuiClient/components/Drawer.tsx at lines 79-87, then update
ChatRow’s title-color ternary at lines 631-679 to check failed before settled.
No direct change is needed at Drawer.tsx lines 983-1010 because the shared
chatStatusDot fix aligns MiniDrawer, or at RightPane.tsx lines 202-221 because
its existing failed-before-settled order is the reference.
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 14189-14200: Update the session mapping that assigns
lastActivityAt to select the maximum parsed timestamp among
attentionRequestedAt, settledAt, lastTurnFailedAt, endedAt, and startedAt,
matching the adjacent refresh path. Replace the fixed nil-coalescing priority
chain while leaving the other session fields unchanged.
In `@apps/ios/ADE/Views/Work/WorkRootComponents.swift`:
- Around line 1169-1171: Update the settled branch in the work-session preview
formatting logic around workSessionPreviewText to use the file’s established
Title Case, no-prefix-label style instead of returning the lowercase “done:”
prefix. Preserve the existing note text and unsettled behavior.
In `@apps/ios/ADE/Views/Work/WorkSessionCanonicalState.swift`:
- Around line 120-126: The settled-state gate in WorkSessionCanonicalState must
not override persisted chat failures during idle. Before returning .settled from
the declared-settle condition, check the chat-specific lastTurnFailedAt marker
and allow failure handling to win; preserve the existing settle behavior for
non-failed chats and non-chat sessions.
In `@apps/ios/ADE/Views/Work/WorkSessionGrouping.swift`:
- Around line 494-511: Update the phase grouping switch in the session grouping
logic so `.ready` and `.idle` are treated as calm sessions rather than appended
to `needsInput`; place them in the appropriate non-attention bucket while
preserving `session.pinned` handling so pinned calm sessions remain in `pinned`.
Keep `.needsYou` as the only phase in that branch that enters the “Your move”
bucket.
---
Outside diff comments:
In `@apps/desktop/src/main/services/pty/ptyService.ts`:
- Around line 4348-4384: Update writeBySessionId() and writeTerminal() to mirror
write()’s attention-clearing behavior: when the target entry has
attentionRequested, reset it and call
sessionService.clearAttentionRequest(entry.sessionId) before writing input.
Preserve the existing write behavior and apply this consistently to both
alternate session-input paths.
---
Nitpick comments:
In `@apps/ade-cli/src/services/push/pushPublisherService.test.ts`:
- Around line 826-855: Update the test “publishes explicit CLI attention
requests through the gated question path” to seed a pending approval request
before calling handleSessionAttentionRequested. After advancing timers, assert
that only the question notification remains and the Live Activity run contains
no approval item, while preserving the existing question payload assertions.
In `@apps/ios/ADE/Views/Work/WorkRootComponents.swift`:
- Around line 1153-1176: Update WorkSessionRowRenderSignature to store the
computed CanonicalSessionState and its badge once during initialization, then
change WorkSessionRow’s canonicalState, capsuleBadge, and isSettled accessors to
reuse renderSignature’s stored values instead of calling
workCanonicalSessionState again. Preserve the existing rendering behavior and
rowPreviewSource logic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: af5cfcce-3e95-4cfc-b3a1-c6cf44c836fa
⛔ Files ignored due to path filters (8)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/PRD.mdis excluded by!docs/**docs/features/ade-code/README.mdis excluded by!docs/**docs/features/agents/README.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/ui-surfaces.mdis excluded by!docs/**
📒 Files selected for processing (72)
apps/ade-cli/README.mdapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/services/push/pushPublisherService.test.tsapps/ade-cli/src/services/push/pushPublisherService.tsapps/ade-cli/src/services/sync/rosterBuilder.test.tsapps/ade-cli/src/services/sync/rosterBuilder.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/tuiClient/__tests__/Drawer.test.tsxapps/ade-cli/src/tuiClient/__tests__/RightPane.test.tsxapps/ade-cli/src/tuiClient/__tests__/adeApi.test.tsapps/ade-cli/src/tuiClient/__tests__/commands.test.tsapps/ade-cli/src/tuiClient/adeApi.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/closedCliSessions.tsapps/ade-cli/src/tuiClient/commands.tsapps/ade-cli/src/tuiClient/components/Drawer.tsxapps/ade-cli/src/tuiClient/components/RightPane.tsxapps/desktop/src/main/services/__tests__/diskFullIncident.integration.test.tsapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/claudeAssistantTextDedup.test.tsapps/desktop/src/main/services/chat/claudeQueryLifecycle.test.tsapps/desktop/src/main/services/chat/claudeSubagentResultGate.test.tsapps/desktop/src/main/services/chat/claudeTaskTodos.test.tsapps/desktop/src/main/services/chat/cursorSdkSystemPrompt.test.tsapps/desktop/src/main/services/chat/cursorSdkSystemPrompt.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/lanes/laneListSnapshotService.tsapps/desktop/src/main/services/pty/ptyService.test.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/main/services/sessions/sessionService.test.tsapps/desktop/src/main/services/sessions/sessionService.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/main/services/usage/usageStatsStore.tsapps/desktop/src/main/services/usage/usageTrackingService.test.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/app/AppShell.tsxapps/desktop/src/renderer/components/lanes/useLaneWorkSessions.tsapps/desktop/src/renderer/components/terminals/SessionCard.test.tsxapps/desktop/src/renderer/components/terminals/SessionCard.tsxapps/desktop/src/renderer/components/terminals/SessionContextMenu.tsxapps/desktop/src/renderer/components/terminals/SessionListPane.test.tsxapps/desktop/src/renderer/components/terminals/SessionListPane.tsxapps/desktop/src/renderer/components/terminals/TerminalsPage.tsxapps/desktop/src/renderer/components/terminals/useWorkSessions.test.tsapps/desktop/src/renderer/components/terminals/useWorkSessions.tsapps/desktop/src/renderer/lib/terminalAttention.test.tsapps/desktop/src/renderer/lib/terminalAttention.tsapps/desktop/src/renderer/state/appStore.tsapps/desktop/src/shared/adeCliGuidance.test.tsapps/desktop/src/shared/adeCliGuidance.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/sessionCanonicalState.test.tsapps/desktop/src/shared/sessionCanonicalState.tsapps/desktop/src/shared/types/sessions.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Models/RemoteRosterModels.swiftapps/ios/ADE/Services/Database.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Views/AttentionDrawer/AttentionDrawerModel.swiftapps/ios/ADE/Views/Work/WorkRootComponents.swiftapps/ios/ADE/Views/Work/WorkSessionCanonicalState.swiftapps/ios/ADE/Views/Work/WorkSessionGrouping.swiftapps/ios/ADETests/ADETests.swiftapps/ios/ADETests/AttentionDrawerModelTests.swift
| // 2. Declared settle — honored only AT REST, mirroring desktop: a settled | ||
| // chat woken by scheduled work shows green while the turn streams, then | ||
| // re-settles at idle (settledAt survives background wakes; only user | ||
| // activity clears it). | ||
| if !settled.isEmpty && (statusLower != "running" || runtimeLower == "idle") { | ||
| return CanonicalSessionState(phase: .settled, badge: nil) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate target file and related tests/files:"
fd -a 'WorkSessionCanonicalState.swift|.*Roster.*' . | sed 's#^\./##' | head -100
echo
echo "Target outline and relevant lines:"
ast-grep outline apps/ios/ADE/Views/Work/WorkSessionCanonicalState.swift || true
wc -l apps/ios/ADE/Views/Work/WorkSessionCanonicalState.swift
sed -n '1,220p' apps/ios/ADE/Views/Work/WorkSessionCanonicalState.swift
sed -n '220,360p' apps/ios/ADE/Views/Work/WorkSessionCanonicalState.swift
echo
echo "Search for settledAt and lastTurnFailedAt usages:"
rg -n "settledAt|lastTurnFailedAt|turnFailed|failedAt|CanonicalSessionState|running-chat turn failure|re-settles|background" apps/ios -SRepository: arul28/ADE
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Desktop canonical state references:"
rg -n "sessionCanonicalState|lastTurnFailedAt|settledAt|Running chats|turn failure|re-settles|lastTurnFailed.isEmpty|settledAt survives|background" . \
--glob '!**/node_modules/**' --glob '!**/.git/**' \
| rg "sessionCanonicalState|lastTurnFailedAt|settledAt|Running chats|turn failure|re-settles|lastTurnFailed\.isEmpty|settledAt survives|background" \
| sed -n '1,240p'
echo
echo "Desktop candidate files:"
git ls-files | rg 'apps/desktop/src/shared/sessionCanonicalState\.ts|sessionCanonicalState|CanonicalState|lastTurnFailedAt|settledAt' | sed -n '1,200p'
echo
echo "Desktop canonical state file if present:"
fpath="$(git ls-files | rg 'sessionCanonicalState\.ts$' | head -n1 || true)"
if [ -n "${fpath:-}" ]; then
wc -l "$fpath"
sed -n '1,260p' "$fpath"
fi
echo
echo "Focused Swift merge/endpoint behavior around settledAt and lastTurnFailedAt:"
sed -n '1740,1825p' apps/ios/ADE/Services/SyncService.swift
sed -n '14140,14305p' apps/ios/ADE/Services/SyncService.swift
rg -n "settledAt\s*=|lastTurnFailedAt\s*=|clear|reset|failed|background" apps/ios/ADE/Services/SyncService.swift apps/ios/ADE/Models -S | sed -n '1,220p'Repository: arul28/ADE
Length of output: 50366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Canonical state files (precise name patterns):"
git ls-files | rg '(^|/).*canonical.*state\.(ts|js|tsx|jsx)$|apps/.*sessionCanonicalState'
echo
echo "Exact desktop canonical state path candidates with sed around core precedence:"
for f in $(git ls-files | rg 'desktop/src/.*/.*CanonicalState|desktop/.*/.*CanonicalState|desktop/.*/sessionCanonicalState' | sed -n '1,20p'); do
echo "===== $f ====="
wc -l "$f"
rg -n "settled|lastTurnFailed|running-chat|background wakes|idle" "$f" -A3 -B3 | sed -n '1,220p'
done
echo
echo "Search all tracked source for exact canonical comments in issue:"
rg -n "settledAt survives|re-settles at idle|Running chats keep status|last turn failed|lastTurnFailed|sessionCanonicalState|canonicalSessionState" . \
--glob '!**/node_modules/**' --glob '!**/.git/**' \
| sed -n '1,260p'
echo
echo "Focused source files containing the relevant canonical implementation/comments:"
rg -l "settledAt survives|re-settles at idle|Running chats keep status|lastTurnFailed|sessionCanonicalState" . \
--glob '!**/node_modules/**' --glob '!**/.git/**' | sed -n '1,40p'Repository: arul28/ADE
Length of output: 20167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Swift canonical tests relevant cases:"
sed -n '1,260p' apps/ios/ADETests/WorkSessionCanonicalStateTests.swift
echo
echo "Desktop canonical implementation core precedence:"
sed -n '90,192p' apps/desktop/src/shared/sessionCanonicalState.ts
echo
echo "Desktop canonical tests relevant cases:"
sed -n '45,60p' apps/desktop/src/shared/sessionCanonicalState.test.ts
sed -n '26,70p' apps/desktop/src/shared/sessionCanonicalState.test.ts
echo
echo "Focused lifecycle/cleared-settle/failure behavior searches:"
rg -n "lastTurnFailedAt|last_turn_failed_at|settledAt|settled_at|clear.*settle|un-settles|settle.*clear|attended_activity|last_activity_at|user activity clears|user activity|next turn starts|turn starts|new activity|clears" . \
--glob '!**/node_modules/**' --glob '!**/.git/**' \
| sed -n '1,300p'
echo
echo "Focused lifecycle/cleared-settle/failure behavior searches in tracked source files:"
for f in $(rg -l "lastTurnFailedAt|settledAt|canonicalSessionState|canonical session state|settle.*clear|clear.*settle|un-settles|new activity clears" . \
--glob '!**/node_modules/**' --glob '!**/.git/**' | sed -n '1,50p'); do
echo "===== $f ====="
rg -n "lastTurnFailedAt|settledAt|canonicalSessionState|canonical session state|settle.*clear|clear.*settle|un-settles|new activity clears|last turn failed|next turn|turn starts|user activity" "$f" -A2 -B2 | sed -n '1,220p'
doneRepository: arul28/ADE
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Focused lifecycle/cleared-settle/failure behavior from smaller tracked sets:"
rg -n "lastTurnFailedAt|last_turn_failed_at|settledAt|settled_at|clear.*settle|settle.*clear|cleared.*settle|un-settles|activity clears|clears settle|next turn|turn starts|last activity clears|last_activity" \
apps/ade-cli src apps/desktop/src/main apps/desktop/src/shared apps/desktop/src/services apps/ios/ADE . \
--glob '!**/node_modules/**' --glob '!**/.git/**' \
| sed -n '1,360p'
echo
echo "Desktop docs around lifecycle columns wording:"
sed -n '1032,1048p' docs/ARCHITECTURE.md
echo
echo "Desktop docs around failed turn statuses wording:"
sed -n '136,154p' docs/features/sync-and-multi-device/push-notifications.md
echo
echo "iOS companion around settledAt wording:"
sed -n '720,730p' docs/features/sync-and-multi-device/ios-companion.md
sed -n '1210,1222p' docs/features/sync-and-multi-device/ios-companion.mdRepository: arul28/ADE
Length of output: 50366
Keep failure winners over idle settle declarations for chats.
The canonical contract treats settledAt as surviving background wakes and being cleared by user activity, while lastTurnFailedAt is only cleared when the next turn starts. A chat can therefore fail at rest, return to runtimeState === "idle" while settledAt still exists, and have the settled gate return .settled before the later chat failure check. Honor the persisted failure marker for chats before applying the idle-settle shortcut.
🤖 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/ios/ADE/Views/Work/WorkSessionCanonicalState.swift` around lines 120 -
126, The settled-state gate in WorkSessionCanonicalState must not override
persisted chat failures during idle. Before returning .settled from the
declared-settle condition, check the chat-specific lastTurnFailedAt marker and
allow failure handling to win; preserve the existing settle behavior for
non-failed chats and non-chat sessions.
There was a problem hiding this comment.
Fixed at the marker level instead: markLastTurnFailed now also clears settled_at (a failure puts the declared outcome in doubt), so settled+failed are mutually exclusive at write time and every surface's precedence agrees by construction — no iOS-only precedence carve-out needed.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Session service outline:"
ast-grep outline apps/desktop/src/main/services/sessions/sessionService.ts --items all --type function,method --match 'markLastTurnFailed|settleSession|clearLastTurnFailed' || true
echo
echo "Marker implementation and nearby lifecycle mutations:"
rg -n -A35 -B15 'markLastTurnFailed\s*[:(]' \
apps/desktop/src/main/services/sessions/sessionService.ts
echo
echo "Regression coverage for failed/settled interaction:"
rg -n -A18 -B8 'markLastTurnFailed|settled_at|settledAt|last_turn_failed_at' \
apps/desktop/src/main/services/sessions/sessionService.test.tsLength of output: 9035
The Comment image shows a stylized “ADE” logo in large white-and-purple block lettering on black, surrounded by layered wavy blue, lavender, and pink contours.
@arul28, I checked the current PR state, and the marker-level fix does not appear to be present yet. markLastTurnFailed at apps/desktop/src/main/services/sessions/sessionService.ts Lines 1325-1332 currently executes:
update terminal_sessions set last_turn_failed_at = ? where id = ?It does not clear settled_at. clearTurnStartMarkers clears both fields, but that is a separate operation and does not establish the invariant when a turn fails.
Please update markLastTurnFailed to clear settled_at in the same mutation (and add a regression assertion for a previously settled session). Once that lands, the marker-level approach is preferable and avoids an iOS-specific precedence exception.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…session state Adds the 'settled' phase to the shared canonical session state machine (declared settle via settledAt, clean-exit auto-settle for PTYs, chat turn-death red via lastTurnFailedAt, ade-chat-ask escalation via attentionRequestedAt), derives a 4-way status bucket (running / your move / ended / settled) shared by the sidebar, lane rollups, and the mobile roster copies, and renders the Settled tier in the Work sidebar: 4th section in status view, per-lane settled tails, by-time bottom section, hollow settled dot, muted rows, Show-settled funnel chip, and statusNote/ask-message as the card's second line replacing the raw output preview (which stays as a detection/search sensor only). Loud 'Needs you' (badge/notifications/tab count) now keys strictly off deterministic asks; resting chats are quiet your-move rows. Refs ADE-125 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns + settle actions Adds terminal_sessions columns settled_at / status_note / attention_requested_at / attention_message / last_turn_failed_at (CRR-safe ALTERs — cr-sqlite alter wrapper handles trigger regen, so they sync to iOS automatically), sessionService lifecycle methods (settle/unsettle single+bulk with newly-settled ids for undo, status note, attention request/clear, turn-failure marker, turn-start clears), activity-driven un-settle folded into the PTY output write, keystroke clearing of CLI attention, chat turn-start marker clears at the shared sendMessage entry and turn-death marking at the normalized done(failed) hook, sessions.settle/unsettle IPC+preload surface, context-menu Settle/Unsettle, section-header Settle-all, bulk-selection Settle, and an 8s undo toast. Canonical settled now only applies at rest so scheduled wakes show green mid-turn and re-settle at idle. Refs ADE-125 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirrors the desktop canonical settled/attention/turn-failure precedence in WorkSessionCanonicalState.swift, decodes the five new terminal_sessions columns (summary + roster + CRR row hydration in Database.swift), adds the status:settled section with 'Your move' labeling, hollow settled dot + 0.7 row opacity, and attention-message/status-note preview priority. Verified with a full iOS simulator xcodebuild (BUILD SUCCEEDED). Refs ADE-125 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rface New CLI subcommands routed over the daemon bridge (ade chat ask "<question>" / note "<text>" / settle [--outcome] / unsettle) backed by session-domain actions (servable even when agentChatService is absent on a headless daemon runtime), server-side scoping that binds them to the calling session's $ADE_CHAT_SESSION_ID, live-PTY escalation through the existing waiting-input runtime-state pipeline, and user notifications via the gated push publisher path used by structured questions (deep links to ade://session/<id>). The shared bootstrap guidance (all 5 SDK chat providers + all 5 tracked CLIs, plus the gated Cursor SDK prompt) now teaches the session status protocol: keep a short ade chat note current, escalate with ade chat ask only when genuinely blocked, declare completion with ade chat settle. Cards linkify PR (#123) and Linear (ABC-123) tokens in status notes and ask messages only — summaries/goals stay plain. Refs ADE-125 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…urns (ADE-130) Background/idle Claude turns deliver tool_use via content_block_start (empty input) + input_json deltas + content_block_stop. The idle handler emitted the tool_call and routed scheduled-work at block_stop but never applied TaskCreate/TaskUpdate/TodoWrite to the runtime task tracker, and the assistant-message path's tracker call was skipped by the emittedToolIds dedupe — so every background-turn task transition was transcribed but dropped, freezing the TASKS pane (and phone) at a prefix of the stream (observed: 0/21 complete vs 16/21 SDK truth). Mirrors the foreground maybeEmitTodoUpdate helper into the idle-turn state (apply-once per tool_use, full input only) and calls it from both the streamed block_stop path and the final assistant-message path. Adds a behavior lock for streamed empty-then-delta task inputs; the two pre-existing scheduled-wake cron test failures reproduce at the branch point and are unrelated. Fixes ADE-130 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
app.setBadgeCount driven by the renderer attention rollup over a new ade.app.setDockBadgeCount IPC — count = sessions deterministically waiting on the user (needs_you), pushed only on change and cleared when attention tracking stops, so a blocked agent reaches the user even with ADE minimized. Refs ADE-125 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dual-review synthesis (correctness/security + maintainability tracks + independent codex review, then a scoped re-review of the applied diff): - ade chat settle now survives the agent's own final assistant text: chat preview writes no longer clear settled_at (PTY output still does via an explicit clearSettled flag) — the feature's core promise was previously undone by the settling turn's last message. - User replies that steer an active turn clear the lifecycle markers (attention/settled/turn-failure); scheduled wakes and empty no-op sends never do, so declared settles survive background wakes and re-settle at rest. - A completed turn clears last_turn_failed_at, so background/scheduled chats that hit one transient error stop showing red forever. - iOS canonical mirror + both lane-rollup bucket copies gained the same at-rest settled gate (a woken settled chat counts as running everywhere) and the rollups now bucket dead chat turns as ended. - Settle/unsettle (single + bulk) route through the bound project runtime for remote projects, with generic session-domain bulk actions mirroring deleteSession's trust posture. - Dock badge is no longer zeroed by navigating off Work routes. - Structural: shared mutateSessionMeta skeleton for session mutators, canonicalInputFromSummary projection replacing five hand-copied 10-field literals, dead sessionIndicatorState/SessionUiState removed (tests ported to dot/bucket), one shared applyClaudeTodoToolUpdate for foreground + idle readers. Verified: desktop focused suites 436 green + full agentChatService suite (only the two pre-existing branch-point cron failures remain), ade-cli 219 green, both typechecks, iOS simulator build SUCCEEDED. Refs ADE-125 ADE-130 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ace parity Analytics (docs/logging.md gate): settle/unsettle recorded as meaningful usage mutations under the existing taxonomy — IPC single+bulk and RPC session-domain actions collapse to work.settleSession/work.unsettleSession (ade_feature_used via the ledger exporter, no new events/properties, ~1 row per user invocation inside existing budgets); status-note writes stay deliberately untracked as agent-frequency mechanics. Mapping and allowlist pinned in usageTrackingService.test. Regression test: scheduled wakes preserve lifecycle markers (settled/ attention/turn-failure) while genuine user sends clear them exactly once. Parity: - Mobile: roster payload now carries the settled-lifecycle fields (additive optional SyncRosterChat fields; rosterBuilder hasColumn guards for older DBs) with iOS decode + attention-drawer wiring and contract tests; sim build SUCCEEDED. - TUI: settled lifecycle surfaced in ade code (Drawer settled grouping, RightPane failed/settled states, namespaced lifecycle slash commands, adeApi wiring) + tests (904 CLI tests green). - CLI: README CLI-surface inventory gains chat ask/note/settle/unsettle. - Docs: 8 internal docs updated (terminals-and-sessions, chat, agents, ade-code, ios-companion, PRD, ARCHITECTURE) for the settled lifecycle, two-tier attention, agent status protocol, and ADE-130 fix. Suite hygiene: no orphaned/skip/no-op tests found in touched folders. Pre-existing environment note: validate-docs flags missing CHANGELOG pages for v1.2.30-v1.2.34 (release-history drift on main, untouched). Refs ADE-125 ADE-130 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3ea7c3a to
3166b5c
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3166b5c3ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Dock badge mirrors the loud tier only; push on change so a blocked | ||
| // agent reaches the user even with the window minimized. | ||
| if (lastDockBadgeCountRef.current !== attention.needsAttentionCount) { | ||
| lastDockBadgeCountRef.current = attention.needsAttentionCount; | ||
| void window.ade?.app?.setDockBadgeCount?.(attention.needsAttentionCount)?.catch?.(() => {}); |
There was a problem hiding this comment.
The new dock update runs inside an attention refresh that returns when document.visibilityState !== "visible" and is disabled entirely outside Work/Lanes. Consequently, if an agent starts or clears a blocking ask while ADE is minimized, backgrounded, or the user is on another tab, setDockBadgeCount is never called and the badge remains stale until the window returns to an adjacent route—the exact minimized-window scenario this badge is intended to cover.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: the visibility early-return is removed, so the attention rollup (and the dock badge it drives) keeps refreshing while the window is hidden/minimized. The refresh remains event-driven + debounced, so hidden cost stays bounded.
| selectedSessions | ||
| .filter((session) => !session.settledAt) | ||
| .map((session) => session.id), |
There was a problem hiding this comment.
When a multi-selection contains an actively running session, this filter includes it solely because settledAt is empty, even though the single-session context menu deliberately hides Settle for active work. The backend writes settledAt, canonical state ignores it while the turn runs, and then the session silently moves to Settled as soon as it becomes idle; the button and undo toast therefore claim it was settled while it continues running and schedule an unexpected later transition.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: bulk settle now targets at-rest rows only (canonical bucket ≠ running) for both the count and the action.
| let canonical = workCanonicalSessionState( | ||
| session: session, | ||
| summary: chatSummaries[session.id] | ||
| ) |
There was a problem hiding this comment.
The status grouping now classifies sessions canonically, but workFilteredSessions still applies normalizedWorkChatSessionStatus, which does not inspect settledAt. An idle settled chat therefore passes the existing Live filter and is then rendered under a Settled section, while there is no Settled status filter at all; filtering and grouping the same row produces contradictory lifecycle results.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: workFilteredSessions now filters with the same canonical phases the grouping uses — settled rows pass only the All filter, and Live/Needs-input/Ended match their sections exactly.
CI: WorkViewArea's partial terminalAttention mock gains the new exports (canonicalInputFromSummary etc.) that broke shard 7. Review fixes (Codex + CodeRabbit, verified against code): - Turn failure un-settles (markLastTurnFailed clears settled_at), making settled/failed mutually exclusive at write time — resolves the cross-surface precedence inconsistencies flagged on the TUI drawer and iOS canonical mirror at the source. - ade chat ask supersedes a pending approval: clears run.itemId, drops the queued approval alert + dedupe fingerprint before enqueueing the question (regression test added). - Dock badge keeps refreshing while the window is hidden (visibility early-return removed; refresh stays event-driven + debounced). - Bulk settle targets at-rest sessions only (canonical bucket). - iOS: workFilteredSessions now filters with canonical phases (settled passes only All), roster lastActivityAt uses the max lifecycle timestamp instead of a priority chain, and the settled preview prefix is title-case. Rejected with replies: Greptile's three cli.ts join-leak P1s (firstPositional splices the sub token; planner tests pin it) and CodeRabbit's ready/idle-bucket objection (intentional two-tier design, desktop parity). Verified: both typechecks, 91 desktop + 43 push tests green, iOS simulator build SUCCEEDED. Refs ADE-125 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1223c70cf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run.phase = "waiting_for_input"; | ||
| run.detail = request.message; |
There was a problem hiding this comment.
When a chat uses ade chat ask, this sets the publisher run to waiting_for_input, but the user-response path only clears the persisted session markers. The subsequent status: started handler changes the phase only when isTerminalPhase(run.phase) is true, so this explicit waiting phase—and its remote badge/Live Activity—remains active while the response turn runs; steering an active turn may not emit a new start at all. Add a publisher-side resolution signal when clearTurnStartMarkers handles a user message, and cancel any queued question alert at the same time.
Useful? React with 👍 / 👎.
| if (isTrackedCli && runtime.ptyService?.hasLivePty(sessionId)) { | ||
| runtime.ptyService.markSessionAttentionRequested(sessionId); | ||
| runtime.ptyService.setSessionRuntimeState(sessionId, "waiting-input"); |
There was a problem hiding this comment.
For a live tracked CLI, ade chat ask writes waiting-input into the PTY runtime state. The settle mutations clear attention_requested_at and set settled_at, but never reset that PTY state; because canonical state gives waiting-input precedence over settled, choosing Settle or Settle all leaves this row in Needs you until the user types into the CLI. Settling an explicit CLI ask needs to clear the corresponding in-memory attention flag/runtime state as well.
Useful? React with 👍 / 👎.
| const renderLaneSessionLists = (laneKey: string, list: TerminalSessionSummary[]) => { | ||
| const active = list.filter((session) => !settledIdSet.has(session.id)); | ||
| const settled = showSettled ? list.filter((session) => settledIdSet.has(session.id)) : []; | ||
| const settledCollapsed = workCollapsedSectionIds.includes(`settled:${laneKey}`); |
There was a problem hiding this comment.
The default organization is by-lane, but fresh state marks only status:settled as collapsed. Each lane tail checks a different dynamic key (settled:<laneId>), which is absent from fresh and migrated state, so every lane's settled rows render expanded by default instead of costing one quiet header line. Projects with substantial history will therefore open with all completed work occupying the sidebar; make lane tails default to collapsed while still persisting explicit expansion.
Useful? React with 👍 / 👎.
| case .needsInput: | ||
| guard !isArchived && status == "awaiting-input" else { return false } | ||
| guard !isArchived, phase == .needsYou || phase == .ready || phase == .idle else { return false } |
There was a problem hiding this comment.
For the iOS filter still labeled Needs input, this predicate now admits every ordinary resting chat (ready) and idle CLI in addition to genuine needsYou sessions. As a result, selecting the warning-oriented filter can show many rows that require no input and disagree with the separately computed needs-input count; either restrict this filter to .needsYou or rename it and its count semantics to Your move.
AGENTS.md reference: AGENTS.md:L59-L61
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/main/services/chat/agentChatService.ts (1)
32703-32732: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
clearUserTurnMarkers()is only wired into two ofsendMessage()'s paths —messageSession()'s directsteer()calls bypass it entirely.The stated intent is that a user message clears settled/attention/turn-failure markers "whether it starts a fresh turn OR steers an active one." That's true only when steering happens through
sendMessage()'srouteActiveToSteeroption (line 32715-32718).messageSession()— the natural entry point for the newade chat ask/note/settle/unsettlelifecycle commands — callssteer(...)directly in at least two places that never go throughsendMessage():
- the
steerTargetbranch (normalizedKind === "queue", or"auto"/non-queueing"wake"while active)- the Claude "interrupt-replace" active branch
Since
steer()itself never callsclearTurnStartMarkers, a queued message or an interrupt-replace against an active Claude turn issued viamessageSessionwon't un-settle the thread — which seems to defeat the purpose of this feature for exactly the surfaces (CLI/agent lifecycle actions) most likely to drive it.Consider hoisting the marker-clearing check into a small shared helper and calling it from
messageSession's directsteer()call sites too (still gated on!metadata?.scheduledWake).♻️ Sketch of a shared helper
- const clearUserTurnMarkers = (): void => { - if (!args.metadata?.scheduledWake) { - sessionService.clearTurnStartMarkers(args.sessionId); - } - }; + // Shared with messageSession()'s direct steer() call sites. + const clearUserTurnMarkers = () => clearUserTurnMarkersIfNotWake(args.sessionId, args.metadata);Then in
messageSession(), before each directsteer(...)call for a genuinely user-driven kind ("queue", or"auto"/"wake"while active, or the Claude "interrupt-replace" active branch):+ if (!metadata?.scheduledWake) { + sessionService.clearTurnStartMarkers(sessionId); + } const result = await steer({🤖 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/desktop/src/main/services/chat/agentChatService.ts` around lines 32703 - 32732, Extend the shared user-turn marker clearing used by sendMessage() to messageSession()’s direct steer() paths. Invoke it before the steerTarget queue/active auto-or-wake routing and the Claude interrupt-replace active branch, while preserving the !metadata?.scheduledWake guard; do not clear markers for scheduled wakes or non-user-driven steering.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/terminals/SessionCard.tsx (1)
368-370: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant canonical-state computation.
sessionCanonicalUiState(sessionAttentionInput)is computed directly at Line 369, andsessionCapsuleBadge(sessionAttentionInput)at Line 370 recomputes it internally (sessionCapsuleBadgejust returnssessionCanonicalUiState(session).badge). Compute once and derive bothphaseandbadgefrom the same result.♻️ Proposed dedup
- const sessionAttentionInput = canonicalInputFromSummary(session); - const canonicalPhase = sessionCanonicalUiState(sessionAttentionInput).phase; - const capsuleBadge = sessionCapsuleBadge(sessionAttentionInput); + const sessionAttentionInput = canonicalInputFromSummary(session); + const canonicalUiState = sessionCanonicalUiState(sessionAttentionInput); + const canonicalPhase = canonicalUiState.phase; + const capsuleBadge = canonicalUiState.badge;🤖 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/desktop/src/renderer/components/terminals/SessionCard.tsx` around lines 368 - 370, Update the SessionCard computation to call sessionCanonicalUiState(sessionAttentionInput) once, store its result, and derive both canonicalPhase and capsuleBadge from that shared state instead of calling sessionCapsuleBadge.
🤖 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/desktop/src/renderer/components/terminals/SessionCard.tsx`:
- Around line 233-277: The LinkifiedPreviewLine interactive spans are nested
inside the session-card button, creating invalid interactive markup and harming
keyboard/accessibility behavior. Restructure the SessionCard layout so the
preview row containing LinkifiedPreviewLine is outside the outer button, leaving
only the title/selectable card area within the button while preserving
independent PR and Linear activation.
---
Outside diff comments:
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 32703-32732: Extend the shared user-turn marker clearing used by
sendMessage() to messageSession()’s direct steer() paths. Invoke it before the
steerTarget queue/active auto-or-wake routing and the Claude interrupt-replace
active branch, while preserving the !metadata?.scheduledWake guard; do not clear
markers for scheduled wakes or non-user-driven steering.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/terminals/SessionCard.tsx`:
- Around line 368-370: Update the SessionCard computation to call
sessionCanonicalUiState(sessionAttentionInput) once, store its result, and
derive both canonicalPhase and capsuleBadge from that shared state instead of
calling sessionCapsuleBadge.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 76ca5d39-9cc4-4550-a111-fabf3eb8addf
📒 Files selected for processing (53)
apps/ade-cli/README.mdapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/services/push/pushPublisherService.test.tsapps/ade-cli/src/services/push/pushPublisherService.tsapps/ade-cli/src/services/sync/rosterBuilder.test.tsapps/ade-cli/src/services/sync/rosterBuilder.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/tuiClient/__tests__/Drawer.test.tsxapps/ade-cli/src/tuiClient/__tests__/RightPane.test.tsxapps/ade-cli/src/tuiClient/__tests__/adeApi.test.tsapps/ade-cli/src/tuiClient/__tests__/commands.test.tsapps/ade-cli/src/tuiClient/adeApi.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/closedCliSessions.tsapps/ade-cli/src/tuiClient/commands.tsapps/ade-cli/src/tuiClient/components/Drawer.tsxapps/ade-cli/src/tuiClient/components/RightPane.tsxapps/desktop/src/main/services/__tests__/diskFullIncident.integration.test.tsapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/claudeAssistantTextDedup.test.tsapps/desktop/src/main/services/chat/claudeQueryLifecycle.test.tsapps/desktop/src/main/services/chat/claudeSubagentResultGate.test.tsapps/desktop/src/main/services/chat/claudeTaskTodos.test.tsapps/desktop/src/main/services/chat/cursorSdkSystemPrompt.test.tsapps/desktop/src/main/services/chat/cursorSdkSystemPrompt.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/lanes/laneListSnapshotService.tsapps/desktop/src/main/services/pty/ptyService.test.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/main/services/sessions/sessionService.test.tsapps/desktop/src/main/services/sessions/sessionService.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/main/services/usage/usageStatsStore.tsapps/desktop/src/main/services/usage/usageTrackingService.test.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/app/AppShell.tsxapps/desktop/src/renderer/components/lanes/useLaneWorkSessions.tsapps/desktop/src/renderer/components/terminals/SessionCard.test.tsxapps/desktop/src/renderer/components/terminals/SessionCard.tsxapps/desktop/src/renderer/components/terminals/SessionContextMenu.tsxapps/desktop/src/renderer/components/terminals/SessionListPane.test.tsxapps/desktop/src/renderer/components/terminals/SessionListPane.tsxapps/desktop/src/renderer/components/terminals/TerminalsPage.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsxapps/desktop/src/renderer/components/terminals/useWorkSessions.test.tsapps/desktop/src/renderer/components/terminals/useWorkSessions.ts
🚧 Files skipped from review as they are similar to previous changes (44)
- apps/desktop/src/main/services/chat/cursorSdkSystemPrompt.test.ts
- apps/desktop/src/main/services/chat/agentChatService.test.ts
- apps/desktop/src/main/services/chat/claudeSubagentResultGate.test.ts
- apps/ade-cli/src/tuiClient/tests/commands.test.ts
- apps/ade-cli/README.md
- apps/desktop/src/main/services/tests/diskFullIncident.integration.test.ts
- apps/desktop/src/main/services/chat/claudeTaskTodos.test.ts
- apps/desktop/src/main/services/usage/usageTrackingService.test.ts
- apps/desktop/src/main/services/chat/claudeAssistantTextDedup.test.ts
- apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
- apps/ade-cli/src/tuiClient/tests/RightPane.test.tsx
- apps/desktop/src/renderer/components/terminals/SessionCard.test.tsx
- apps/desktop/src/main/services/ipc/registerIpc.ts
- apps/desktop/src/main/services/state/kvDb.ts
- apps/ade-cli/src/tuiClient/tests/Drawer.test.tsx
- apps/desktop/src/main/services/pty/ptyService.test.ts
- apps/desktop/src/preload/global.d.ts
- apps/desktop/src/main/services/usage/usageStatsStore.ts
- apps/ade-cli/src/tuiClient/closedCliSessions.ts
- apps/desktop/src/main/services/chat/claudeQueryLifecycle.test.ts
- apps/desktop/src/renderer/components/terminals/TerminalsPage.tsx
- apps/desktop/src/renderer/components/terminals/useWorkSessions.test.ts
- apps/desktop/src/main/services/sessions/sessionService.test.ts
- apps/desktop/src/main/services/chat/cursorSdkSystemPrompt.ts
- apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts
- apps/desktop/src/main/services/lanes/laneListSnapshotService.ts
- apps/desktop/src/renderer/components/app/AppShell.tsx
- apps/ade-cli/src/services/push/pushPublisherService.ts
- apps/desktop/src/main/services/adeActions/registry.test.ts
- apps/ade-cli/src/tuiClient/components/Drawer.tsx
- apps/desktop/src/main/services/adeActions/registry.ts
- apps/ade-cli/src/tuiClient/app.tsx
- apps/ade-cli/src/services/sync/rosterBuilder.ts
- apps/ade-cli/src/adeRpcServer.ts
- apps/ade-cli/src/tuiClient/commands.ts
- apps/ade-cli/src/tuiClient/components/RightPane.tsx
- apps/desktop/src/renderer/components/terminals/SessionContextMenu.tsx
- apps/desktop/src/renderer/components/terminals/SessionListPane.test.tsx
- apps/desktop/src/preload/preload.ts
- apps/ade-cli/src/tuiClient/adeApi.ts
- apps/desktop/src/renderer/components/terminals/useWorkSessions.ts
- apps/desktop/src/main/services/pty/ptyService.ts
- apps/desktop/src/main/services/sessions/sessionService.ts
- apps/desktop/src/renderer/components/terminals/SessionListPane.tsx
| function LinkifiedPreviewLine({ | ||
| text, | ||
| onOpenPr, | ||
| }: { | ||
| text: string; | ||
| onOpenPr: (prNumber: number) => void; | ||
| }) { | ||
| return text.split(PREVIEW_LINK_TOKEN).map((part, index) => { | ||
| const prMatch = PR_TOKEN.exec(part); | ||
| const linearIssue = LINEAR_TOKEN.test(part) ? part : null; | ||
| if (!prMatch && !linearIssue) return part; | ||
| const activate = () => { | ||
| if (prMatch) { | ||
| onOpenPr(Number.parseInt(prMatch[1]!, 10)); | ||
| } else if (linearIssue) { | ||
| requestLinearIssueQuickView({ | ||
| issueIdentifier: linearIssue, | ||
| source: "manual", | ||
| }); | ||
| } | ||
| }; | ||
| return ( | ||
| <span | ||
| key={`${part}:${index}`} | ||
| role="link" | ||
| tabIndex={0} | ||
| className="cursor-pointer text-muted-fg/70 hover:underline" | ||
| title={prMatch ? `Open PR ${part}` : `Open Linear issue ${part}`} | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| activate(); | ||
| }} | ||
| onKeyDown={(event) => { | ||
| if (event.key !== "Enter" && event.key !== " ") return; | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| activate(); | ||
| }} | ||
| > | ||
| {part} | ||
| </span> | ||
| ); | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src/renderer/components/terminals/SessionCard.tsx"
if [ ! -f "$file" ]; then
echo "missing $file" >&2
git ls-files | rg 'SessionCard\.tsx$' || true
exit 0
fi
echo "== outline =="
ast-grep outline "$file" --view compact || true
echo "== relevant sections =="
sed -n '1,110p' "$file"
sed -n '620,670p' "$file"
sed -n '420,480p' "$file"
sed -n '880,920p' "$file"
echo "== interactive/nested focus checks =="
python3 - <<'PY'
from pathlib import Path
import re
p = Path("apps/desktop/src/renderer/components/terminals/SessionCard.tsx")
s = p.read_text()
for name, patterns in {
"LinkifiedPreviewLine": [
r'<span\s+[^>]*role="link"',
r'<a\s+',
r'<button\s+',
r'tabIndex=\{0\}',
r'<span\s+[^>]*role="button"',
r'<input\s+',
r'<textarea\s+',
],
}:
print(f"== {name} ==")
for pat in patterns:
print(pat, len(re.findall(pat, s, re.I)))
PYRepository: arul28/ADE
Length of output: 8902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src/renderer/components/terminals/SessionCard.tsx"
if [ ! -f "$file" ]; then
echo "missing $file" >&2
exit 0
fi
echo "== LinkifiedPreviewLine section =="
grep -n "function LinkifiedPreviewLine\\|const PR_TOKEN\\|const LINEAR_TOKEN\\|requestLinearIssueQuickView"
sed -n '200,290p' "$file"
sed -n '638,660p' "$file"
sed -n '435,470p' "$file"
echo "== spawn-lineage invalid nesting section =="
grep -n "nested interactive control is invalid HTML\\|spawn-lineage\\|role=\\\"button\\\"\\|tabIndex"
sed -n '900,940p' "$file"
echo "== focused token occurrences =="
python3 - <<'PY'
from pathlib import Path
import re
s = Path("apps/desktop/src/renderer/components/terminals/SessionCard.tsx").read_text()
for pat in [
r'<span\s+[^>]*role=["\']link["\'][^>]*>',
r'tabIndex=\{0\}',
r'<button\s',
r'<a\s',
r'<span\s+[^>]*role=["\']button["\'][^>]*>',
]:
print(f"{pat}: {len(re.findall(pat, s, re.I))}")
PYRepository: arul28/ADE
Length of output: 183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src/renderer/components/terminals/SessionCard.tsx"
if [ ! -f "$file" ]; then
echo "missing $file" >&2
exit 0
fi
echo "== LinkifiedPreviewLine section =="
grep -n "function LinkifiedPreviewLine\\|const PR_TOKEN\\|const LINEAR_TOKEN\\|requestLinearIssueQuickView" "$file" || true
sed -n '200,290p' "$file"
sed -n '638,660p' "$file"
sed -n '435,470p' "$file"
echo "== spawn-lineage invalid nesting section =="
grep -n "nested interactive control is invalid HTML\\|spawn-lineage\\|role=\\\"button\\\"\\|tabIndex" "$file" || true
sed -n '900,940p' "$file"
echo "== focused token occurrences =="
python3 - <<'PY'
from pathlib import Path
import re
s = Path("apps/desktop/src/renderer/components/terminals/SessionCard.tsx").read_text()
for pat in [
r'<span\s+[^>]*role=["\']link["\'][^>]*>',
r'tabIndex=\{0\}',
r'<button\s',
r'<a\s',
r'<span\s+[^>]*role=["\']button["\'][^>]*>',
]:
print(f"{pat}: {len(re.findall(pat, s, re.I))}")
PYRepository: arul28/ADE
Length of output: 6464
Avoid nesting interactive link spans inside the session-card <button>.
LinkifiedPreviewLine renders <span role="link" tabIndex={0}> inside the preview line, but that line is rendered inside the card’s outer <button>, which is HTML-invalid and can make assistive technology flatten or skip these actions. Since these PR/Linear links need independent keyboard access, move the row outside the button or restructure the card so only the title/select area remains the <button>.
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 241-241: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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/desktop/src/renderer/components/terminals/SessionCard.tsx` around lines
233 - 277, The LinkifiedPreviewLine interactive spans are nested inside the
session-card button, creating invalid interactive markup and harming
keyboard/accessibility behavior. Restructure the SessionCard layout so the
preview row containing LinkifiedPreviewLine is outside the outer button, leaving
only the title/selectable card area within the button while preserving
independent PR and Linear activation.
Source: Path instructions
useLaneWorkSessions.test.ts was the remaining partial factory mock missing canonicalInputFromSummary (shard 7). All three factory mocks of the module now provide the new exports. Refs ADE-125 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Fixes ADE-125
Linked Linear issues
Summary by CodeRabbit
New Features
/chat ask,/chat note,/chat settle,/chat unsettle, plus matching CLI chat lifecycle helpers (with--sessiontargeting).#123) and Linear-style tokens.Bug Fixes
Greptile Summary
This PR adds a settled session lifecycle and two-tier attention states across ADE surfaces. The main changes are:
ade chat ask,note,settle, andunsettlecommands.Confidence Score: 4/5
Mostly safe to merge after fixing the sidebar badge count.
Core scoping, persistence, canonical-state, and cross-surface rendering changes are coherent. One contained bug remains where settled spawned sessions can still count as live on their parent badge.
apps/desktop/src/renderer/components/terminals/SessionListPane.tsx
What T-Rex did
Important Files Changed
ade chat ask/note/settle/unsettleplanning with caller-scoped defaults and explicit--sessionsupport.Sequence Diagram
sequenceDiagram participant Agent as Agent / TUI / CLI participant RPC as ADE RPC action bridge participant Session as sessionService participant Push as pushPublisherService participant UI as Work sidebar / iOS roster Agent->>RPC: ade chat ask/note/settle/unsettle RPC->>RPC: Scope session action to caller unless CTO RPC->>Session: Persist lifecycle marker alt explicit ask RPC->>Push: handleSessionAttentionRequested Push-->>UI: Needs-you notification / Live Activity end UI->>Session: list sessions / sync roster Session-->>UI: lifecycle fields UI->>UI: canonicalSessionState + status bucketComments Outside Diff (1)
apps/desktop/src/renderer/components/terminals/SessionListPane.tsx, line 536-537 (link)liveChildrenByParentIdstill treats any child withstatus === "running"as live, but settled chat sessions keep the persistedrunningstatus and are only quieted bysettledAtthrough the canonical bucket. When a spawned child is settled, the parent badge still shows it as live even though the row moved to the settled tier.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (4): Last reviewed commit: "fix(ci): complete the partial terminalAt..." | Re-trigger Greptile