diff --git a/docs/team-management/member-work-sync-review-obligation-plan.md b/docs/team-management/member-work-sync-review-obligation-plan.md new file mode 100644 index 00000000..4f6fdcbf --- /dev/null +++ b/docs/team-management/member-work-sync-review-obligation-plan.md @@ -0,0 +1,2078 @@ +# Member Work Sync Review Obligation Plan + +**Status:** design proposal, ready for implementation review +**Scope:** member-work-sync, review lifecycle, member nudges, stuck review pickup +**Primary repo:** `claude_team` +**Related controller boundary:** `agent-teams-controller` +**Recommended option:** Work-sync review pickup obligations +**Rating:** 🎯 9 🛡️ 9 🧠 8 +**Estimated size:** 850-1250 changed lines with tests for the reliable version. The smaller 350-550 LOC version is only safe for OpenCode-first rollout and does not fully fix the live Anthropic/Alice class. + +--- + +## 1. Summary + +The current `member-work-sync` feature already detects review work in the actionable agenda. It correctly detected the live `ember-collective` Alice case as: + +```json +{ + "state": "needs_sync", + "providerId": "anthropic", + "agendaItems": [ + { + "kind": "review", + "priority": "review_requested", + "taskId": "7142f765-76e5-4532-8a37-e228b841a6ed" + } + ] +} +``` + +But the current system does not yet enforce review pickup: + +- Phase 2 nudges are blocked for Anthropic while metrics are not `shadow_ready`. +- The nudge text is generic and report-oriented. +- A `member_work_sync_report(still_working)` can suppress more nudges, but it does not prove that the reviewer called `review_start`. +- `currentReviewCycle.ts` can mix old `review_started` evidence with a newer `review_requested` event, because it does not model a strict review cycle boundary. +- Production delivery wake is OpenCode-only today, and even that wake is fire-and-forget. For Anthropic/native members, inbox insertion alone is not enough evidence that a live member was prompted. +- Review-cycle logic is duplicated across work-sync, controller, stall monitor, and renderer timer fallbacks. If these drift, the system can show a timer, skip a nudge, and still leave the task stuck. + +The recommended fix is not a separate ping watchdog. The recommended fix is a precise extension of work-sync: + +```text +review_requested -> review_pickup_required obligation +review_started -> review_in_progress, handled by timers and stall monitor +review_decision -> obligation gone +status reset -> obligation gone +``` + +Only `review_pickup_required` should be allowed to bypass generic Phase 2 readiness. It should still use all existing anti-spam controls: + +- durable outbox idempotency; +- current agenda revalidation before dispatch; +- active team check; +- provider delivery capability check; +- member busy signal; +- per-member rate limit; +- watchdog cooldown; +- one-shot member nudge keyed by `reviewRequestEventId`, not by the whole agenda fingerprint; +- lead-facing escalation after ignored correction instead of repeated member spam. + +⚠️ Important correction to the earlier plan: do not enable Anthropic review-pickup bypass just because activation says `review_pickup_required`. Enable it only after a provider delivery outcome path is implemented and tested. Otherwise the system can correctly enqueue a nudge that nobody sees promptly. + +--- + +## 2. Decision + +Recommended implementation: + +**Review pickup obligation inside member-work-sync** +🎯 9 🛡️ 9 🧠 8, roughly 850-1250 LOC with tests for the full reliable version. + +Why this is the best fit: + +- It reuses the existing work-sync control plane instead of adding another notification loop. +- It is level-triggered: every dispatch revalidates the current task state. +- It already has durable outbox idempotency, and we can add the missing review-cycle idempotency on top. +- It handles the exact failure class where a reviewer reads a request, answers "duplicate", and never calls `review_start`. +- It avoids interrupting real work because busy signal and rate limiting already sit in the dispatcher. + +Rejected or weaker options: + +1. **Standalone review pickup watchdog** - 🎯 6 🛡️ 6 🧠 5, 180-300 LOC + Easy to add, but it duplicates work-sync and can spam. It would need to rediscover the same lifecycle, busy, idempotency, and rate-limit rules. + +2. **Auto-open `review_start` when review request is delivered/read** - 🎯 6 🛡️ 7 🧠 5, 120-220 LOC + It improves timer visibility but can overcount time. Receiving a message is not the same as actually starting review. + +3. **Only strengthen review request prompt text** - 🎯 7 🛡️ 5 🧠 2, 20-60 LOC + Useful as defense-in-depth, but not reliable. The Alice incident happened because the model reasoned itself out of following the tool protocol. + +--- + +## 3. Live Incident That Motivated This + +Task: + +```text +team: ember-collective +task: #7142f765 +subject: Docs: Workflows (runtime-setup/agent-workflow/code-review/troubleshooting) - EN+RU +owner: jack +reviewer: alice +``` + +Observed task history: + +```text +08:02:35 review_requested reviewer=alice +08:02:43 review_started actor=alice +08:03:25 status_changed completed -> in_progress +08:03:45 status_changed in_progress -> completed +08:04:16 review_requested reviewer=alice +08:04:19 review_approved actor=alice +08:05:19 status_changed completed -> in_progress +08:05:24 status_changed in_progress -> completed +08:05:28 review_requested reviewer=alice +``` + +Current state: + +```text +status: completed +reviewState: review +latest review cycle: requested only, no review_started +reviewIntervals: old closed interval only +``` + +Alice processed the latest message but replied that it was a duplicate. She did not call: + +```text +review_start +review_approve +review_request_changes +``` + +Work-sync then evaluated her as `needs_sync`, but skipped the nudge: + +```json +{ + "event": "nudge_skipped", + "reason": "phase2_not_ready", + "providerId": "anthropic", + "taskRefs": [ + { + "taskId": "7142f765-76e5-4532-8a37-e228b841a6ed", + "displayId": "7142f765" + } + ] +} +``` + +Conclusion: + +```text +work-sync detection works +review pickup enforcement is missing +``` + +--- + +## 4. Current Architecture Facts + +### 4.1 Agenda already includes reviews + +`buildActionableWorkAgenda()` produces `kind: "review"` items when a task is in review workflow and the current reviewer resolves to the member. + +Current shape: + +```ts +items.push({ + ...base, + kind: 'review', + priority: 'review_requested', + reason: 'current_cycle_review_assigned', + evidence: { + status: task.status, + owner, + reviewer: memberName, + reviewState: task.reviewState, + historyEventIds: reviewOwner.historyEventIds, + }, +}); +``` + +This is good for basic agenda computation, but insufficient for enforcement because the item does not say whether pickup is still required or review is already started. + +### 4.2 Current review cycle resolver is too loose + +Current resolver: + +```ts +const latestStarted = [...historyEvents].reverse().find((event) => event.type === 'review_started'); +const latestRequested = [...historyEvents] + .reverse() + .find((event) => event.type === 'review_requested'); +``` + +Problem: + +```text +old review_started can be returned together with a newer review_requested +``` + +This was visible in the live Alice status. The evidence had both: + +```text +old review_started id +latest review_requested id +``` + +That should not happen for a strict cycle model. + +### 4.3 Controller review lifecycle has better boundaries + +The controller already treats these as review cycle boundaries: + +```js +if ( + e.type === 'review_changes_requested' || + e.type === 'review_approved' || + (e.type === 'status_changed' && + (e.to === 'in_progress' || e.to === 'pending' || e.to === 'deleted')) || + e.type === 'task_created' +) { + return null; +} +``` + +Work-sync should match this boundary logic, otherwise renderer/controller/work-sync can disagree about the current cycle. + +### 4.4 Work-sync reports are leases, not task transitions + +The task protocol explicitly says: + +```text +member_work_sync_status and member_work_sync_report are only for reporting whether you have seen the current actionable-work agenda. +They do NOT start, complete, approve, or comment on tasks. +Never use member_work_sync_report instead of task_start, task_complete, review_approve, review_request_changes, task_set_clarification, or task_add_comment. +``` + +This means a review pickup nudge must not treat `still_working` as a successful review pickup. It can only be a bounded lease that prevents immediate repeated nudging. + +### 4.5 Existing anti-spam infrastructure is useful + +Current `MemberWorkSyncNudgeDispatcher` already revalidates before delivery: + +```text +team active +status exists +agenda recomputed +decision is still needs_sync +agenda fingerprint still matches +phase activation allows it +rate limit allows it +busy signal allows it +watchdog cooldown allows it +``` + +This is the right place to add review-specific activation. The wrong place is a direct notification inside `review_request`. + +### 4.6 Delivery wake is currently provider-asymmetric + +Production composition wires `nudgeDeliveryWake` only for OpenCode: + +```ts +nudgeDeliveryWake: { + schedule: (input) => { + if (input.providerId !== 'opencode') { + return; + } + teamProvisioningService.scheduleOpenCodeMemberInboxDeliveryWake(...); + }, +} +``` + +The normal teammate inbox relay in `src/main/ipc/teams.ts` is explicitly disabled: + +```text +Teammate inbox relay DISABLED (2026-03-23). +Codex/Claude teammates read their own inbox files directly via fs.watch. +Relaying through the lead caused multiple bugs. +``` + +This makes the live Alice case more subtle: + +```text +work-sync can detect the stuck Anthropic review +work-sync can insert an inbox row +but without a delivery outcome path, the live member may not process it soon +``` + +Required conclusion: + +```text +Review pickup bypass needs a delivery-outcome capability gate. +OpenCode can use the existing wake. +Anthropic/native needs either a proven fs-watch delivery outcome path or a new narrow delivery path. +Do not resurrect the old lead relay blindly. +``` + +### 4.7 Task impact routing can miss review owners + +`MemberWorkSyncTaskImpactResolver` computes `taskWorkflowColumn` from kanban-aware state, but then resolves review owner using raw `task.reviewState`: + +```ts +const taskWorkflowColumn = getTeamTaskWorkflowColumn({ + ...task, + ...(taskKanbanColumn ? { kanbanColumn: taskKanbanColumn } : {}), +}); + +const reviewOwner = + taskWorkflowColumn === 'review' + ? resolveCurrentReviewOwner({ + reviewState: task.reviewState, + kanbanReviewer: kanban.tasks[task.id]?.reviewer ?? null, + historyEvents: task.historyEvents, + }) + : null; +``` + +If kanban says the task is in review but persisted `task.reviewState` is stale or missing, full agenda recompute can still see the review, while task-impact routing may not wake the reviewer. The plan must fix this by passing the workflow column into the review-cycle resolver: + +```ts +resolveCurrentReviewCycle({ + reviewState: taskWorkflowColumn, + kanbanReviewer, + historyEvents, +}); +``` + +This is not a cleanup. It is required for reliable triggering. + +### 4.8 Review-cycle logic is duplicated and can drift + +Current related logic exists in several places: + +- `src/features/member-work-sync/core/domain/currentReviewCycle.ts` +- `agent-teams-controller/src/internal/agenda.js` +- `src/main/services/team/stallMonitor/reviewerResolution.ts` +- `src/main/services/team/stallMonitor/TeamTaskStallPolicy.ts` +- renderer review timer fallback logic + +These components do not need identical output, but they must agree on the same lifecycle boundaries: + +```text +review_requested starts or replaces current review request +review_started only belongs to the current request if it happens after that request +review_approved closes the cycle +review_changes_requested closes the cycle +status_changed -> in_progress/pending/deleted closes the cycle +task_created resets history +``` + +Implementation should add shared fixtures or a shared helper where imports are practical. If controller JS cannot directly import TS shared code, keep the implementations separate but test the same event tables on both sides. + +### 4.9 Nudge payload metadata is too thin for robust review intent + +Current inbox messages preserve `messageKind`, but there is no structured `intent`, `intentKey`, or `reviewRequestEventId` in `InboxMessage` / `SendMessageRequest`. + +That means a review-pickup nudge can only be recognized by generic `member_work_sync_nudge` kind or brittle text matching. This is weak for: + +- OpenCode wrapper wording; +- one-shot per review request; +- lead escalation after ignored pickup; +- debugging delivered vs superseded rows. + +Reliable implementation should extend the work-sync payload and inbox/sent-message persistence with structured intent metadata: + +```ts +workSyncIntent?: 'agenda_sync' | 'review_pickup'; +workSyncIntentKey?: string; // review-pickup: +workSyncReviewRequestEventIds?: string[]; +``` + +This costs more lines, but it avoids depending on prompt text as a machine-readable contract. + +### 4.10 Outbox `delivered` currently means inbox row inserted + +Current dispatcher order is: + +```text +insert inbox row +mark outbox delivered +append nudge_delivered +schedule delivery wake +``` + +That is acceptable for passive sync reminders, but it is too weak for review pickup. If wake scheduling fails after the row is inserted, the outbox is already terminal `delivered`. Then: + +- retry will not happen because `delivered` is terminal; +- rate limit can count a nudge that never reached live input; +- one-shot marker can incorrectly block future repair; +- lead escalation may think the member ignored the correction, when the member never saw it. + +Reliable review pickup needs a stronger definition: + +```text +inbox_persisted = JSON row exists +prompt_accepted = live runtime/provider accepted the prompt or direct member wake +response_proven = runtime saw acceptable proof, such as report or task progress +delivery unavailable = not delivered, audit and lead-escalate +``` + +For review pickup, one-shot should be written at `prompt_accepted`, not at `inbox_persisted`. The review obligation itself is cleared only by task state (`review_start`, `review_approve`, `review_request_changes`) or by a short report lease. + +Implementation options: + +1. **Three-state review-pickup delivery model** - 🎯 9 🛡️ 9 🧠 8, 180-320 LOC + Add explicit `inbox_persisted`, `prompt_accepted`, and `response_proven` metadata/statuses for review pickup. `prompt_accepted` is enough to prevent repeated member nudges; `response_proven` is useful for diagnostics. On prompt failure, keep retryable with `nextAttemptAt`. + +2. **Synchronous review-pickup delivery port** - 🎯 9 🛡️ 8 🧠 7, 140-260 LOC + Add a dedicated port that persists the inbox row and immediately attempts provider delivery. For OpenCode, call the relay path that returns `lastDelivery` instead of only scheduling the watchdog. Mark prompt accepted only when provider delivery says accepted or response pending after accepted prompt. + +3. **Reorder insert -> schedule wake -> mark delivered** - 🎯 6 🛡️ 5 🧠 4, 50-100 LOC + Better than current order, but still weak because `scheduleOpenCodeMemberInboxDeliveryWake()` is a fire-and-forget timer. Scheduling the watchdog is not proof that OpenCode accepted the prompt. + +4. **Keep current order and audit wake failure** - 🎯 5 🛡️ 4 🧠 2, 10-30 LOC + Not reliable. It preserves the exact false-delivered failure mode. + +Recommendation: Option 2 for the first implementation if it can return a real provider outcome. If not, use Option 1 and let the watchdog/relay result transition the item from `inbox_persisted` to `prompt_accepted`. + +### 4.11 Fire-and-forget wake is not delivery proof + +`scheduleOpenCodeMemberInboxDeliveryWake()` currently schedules a watchdog job and returns `void`. It can tell us that a timer was installed, but not whether: + +- the runtime was still active when the timer fired; +- the relay found the message; +- OpenCode accepted the prompt; +- the prompt resulted in response proof; +- the inbox read commit succeeded. + +For generic work-sync, this may be acceptable because the next queue/scheduler pass can keep nudging under rate limits. For review pickup, it is not acceptable because one-shot behavior and lead escalation depend on knowing whether the member was actually prompted. + +Required plan change: + +```text +nudgeDeliveryWake.schedule remains ok for generic nudges +review_pickup uses a delivery-outcome path, not fire-and-forget schedule alone +``` + +Possible API: + +```ts +export interface MemberWorkSyncReviewPickupDeliveryPort { + canDeliver(input: ReviewPickupDeliveryTarget): Promise; + deliver(input: ReviewPickupDeliveryRequest): Promise; +} + +export type ReviewPickupDeliveryOutcome = + | { + ok: true; + state: 'prompt_accepted' | 'response_proven'; + messageId: string; + diagnostics?: string[]; + } + | { ok: false; state: 'capability_absent'; reason: string; diagnostics?: string[] } + | { + ok: false; + state: 'retryable_failure'; + reason: string; + nextAttemptAt?: string; + diagnostics?: string[]; + } + | { ok: false; state: 'terminal_failure'; reason: string; diagnostics?: string[] }; +``` + +### 4.12 Persisted status can be stale after restart + +`MemberWorkSyncDiagnosticsReader.getStatus()` currently returns stored status immediately when it exists. The renderer hook also calls `getStatus`, not `refreshStatus`, for normal display. That means after app restart the UI can briefly or indefinitely show a persisted status that was evaluated before: + +- a new task history event; +- a crash repair; +- a startup scan; +- a provider delivery retry; +- a report lease expiry. + +For generic diagnostics this is tolerable. For review pickup it can hide the fact that no outbox planning has happened yet. + +Required behavior: + +```text +stored status should expose staleness +startup scan should repair/reconcile active members +UI/debugging should be able to tell persisted snapshot from fresh queue-planned status +``` + +Implementation options: + +1. **Staleness-aware `getStatus`** - 🎯 8 🛡️ 8 🧠 5, 80-160 LOC + If stored status is older than a threshold, source revision changed, report lease expired, or app restart marker is newer, return status with `diagnostics: ['status_snapshot_stale']` and enqueue refresh. Do not plan nudges from plain UI reads. + +2. **Renderer calls `refreshStatus` for the panel** - 🎯 7 🛡️ 7 🧠 3, 20-50 LOC + Simpler, but it can turn UI visits into side-effectful reconciliation unless carefully separated from outbox planning. + +3. **Keep current stored-first behavior** - 🎯 4 🛡️ 4 🧠 1, 0 LOC + Not enough for reliable review pickup after crash/restart. + +Recommendation: Option 1. Keep `getStatus` cheap and mostly read-only, but make staleness explicit and enqueue a queue reconciliation when needed. Outbox planning still happens in queue reconciliation, not in the read path. + +--- + +## 5. Core Model + +Add a review obligation concept to the agenda item evidence. + +```ts +export type MemberWorkSyncReviewObligation = 'review_pickup_required' | 'review_in_progress'; + +export interface MemberWorkSyncReviewCycleEvidence { + reviewCycleId: string; + reviewRequestEventId: string; + reviewRequestedAt: string; + reviewStartedEventId?: string; + reviewStartedAt?: string; + reviewStartedBy?: string; + obligation: MemberWorkSyncReviewObligation; + canBypassPhase2: boolean; + diagnostics?: string[]; +} +``` + +Suggested contract extension: + +```ts +export interface MemberWorkSyncActionableWorkItem { + taskId: string; + displayId?: string; + subject: string; + kind: MemberWorkSyncActionableWorkKind; + assignee: string; + priority: MemberWorkSyncActionableWorkPriority; + reason: string; + evidence: { + status: string; + owner?: string; + reviewer?: string; + reviewState?: string; + reviewCycleId?: string; + reviewRequestEventId?: string; + reviewRequestedAt?: string; + reviewStartedEventId?: string; + reviewStartedAt?: string; + reviewStartedBy?: string; + reviewObligation?: MemberWorkSyncReviewObligation; + canBypassPhase2?: boolean; + reviewDiagnostics?: string[]; + historyEventIds?: string[]; + }; +} +``` + +The fingerprint must include this evidence. That gives one stable agenda fingerprint per active review request cycle. + +For nudge idempotency, add a separate intent key that is stable even if the agenda contains other tasks: + +```ts +export interface MemberWorkSyncReviewPickupIntent { + intent: 'review_pickup'; + intentKey: `review-pickup:${string}`; + reviewRequestEventIds: string[]; +} +``` + +Do not use only `agendaFingerprint` for one-shot semantics. Agenda fingerprint can change when another task appears, a subject changes, or evidence formatting changes. The review request event id is the real lifecycle identity. + +--- + +## 6. Review Cycle Resolver + +Replace the loose owner resolver with a strict cycle resolver. + +### 6.1 Desired resolver output + +```ts +export interface CurrentReviewCycle { + reviewer: string; + reviewCycleId: string; + requestEventId: string; + requestedAt: string; + startedEventId?: string; + startedAt?: string; + startedBy?: string; + diagnostics: string[]; + canBypassPhase2: boolean; + obligation: 'review_pickup_required' | 'review_in_progress'; + historyEventIds: string[]; +} +``` + +### 6.2 Resolver algorithm + +Pseudo-code: + +```ts +const REVIEW_CYCLE_BOUNDARY_TYPES = new Set([ + 'task_created', + 'review_approved', + 'review_changes_requested', +]); + +function isStatusReset(event: ReviewHistoryEventLike): boolean { + return ( + event.type === 'status_changed' && + (event.to === 'in_progress' || event.to === 'pending' || event.to === 'deleted') + ); +} + +function getCurrentReviewCycle(input: { + reviewState?: string | null; + kanbanReviewer?: string | null; + historyEvents?: ReviewHistoryEventLike[]; +}): CurrentReviewCycle | null { + if (input.reviewState !== 'review') { + return null; + } + + const events = (input.historyEvents ?? []) + .map((event, index) => ({ event, index })) + .sort((a, b) => compareEventsByTimestampThenIndex(a, b)); + + let request: ReviewHistoryEventLike | null = null; + let requestIndex = -1; + let startedByReviewer: ReviewHistoryEventLike | null = null; + let ambiguousStarted: ReviewHistoryEventLike | null = null; + const diagnostics: string[] = []; + + for (const { event, index } of events) { + if (REVIEW_CYCLE_BOUNDARY_TYPES.has(event.type) || isStatusReset(event)) { + request = null; + requestIndex = -1; + startedByReviewer = null; + ambiguousStarted = null; + diagnostics.length = 0; + continue; + } + + if (event.type === 'review_requested') { + request = event; + requestIndex = index; + startedByReviewer = null; + ambiguousStarted = null; + diagnostics.length = 0; + continue; + } + + if (event.type === 'review_started' && request && index > requestIndex) { + const requestedReviewer = + normalizeMemberName(request.reviewer) || normalizeMemberName(input.kanbanReviewer); + const startedBy = normalizeMemberName(event.actor); + + if (!startedBy) { + diagnostics.push('review_started_actor_missing'); + ambiguousStarted = event; + continue; + } + + if (requestedReviewer && startedBy !== requestedReviewer) { + diagnostics.push('review_started_by_different_member'); + ambiguousStarted = event; + continue; + } + + startedByReviewer = event; + } + } + + if (!request) { + return legacyKanbanFallback(input); + } + + const reviewer = + normalizeMemberName(request.reviewer) || + normalizeMemberName(input.kanbanReviewer) || + normalizeMemberName(startedByReviewer?.actor) || + normalizeMemberName(ambiguousStarted?.actor); + + if (!reviewer) { + return null; + } + + const effectiveStarted = startedByReviewer ?? ambiguousStarted; + const hasStartedEvidence = Boolean(effectiveStarted); + const validStartedByReviewer = startedByReviewer; + + return { + reviewer, + reviewCycleId: request.id ?? `${request.timestamp ?? ''}:${reviewer}`, + requestEventId: request.id ?? '', + requestedAt: request.timestamp ?? '', + ...(effectiveStarted?.id ? { startedEventId: effectiveStarted.id } : {}), + ...(effectiveStarted?.timestamp ? { startedAt: effectiveStarted.timestamp } : {}), + ...(effectiveStarted?.actor ? { startedBy: effectiveStarted.actor } : {}), + diagnostics, + canBypassPhase2: Boolean(request.id) && !hasStartedEvidence && diagnostics.length === 0, + obligation: hasStartedEvidence ? 'review_in_progress' : 'review_pickup_required', + historyEventIds: [request.id, effectiveStarted?.id].filter(Boolean), + }; +} +``` + +Important behavior: + +- A `review_started` before a later `status_changed -> in_progress` must not count. +- A `review_started` before a later `review_approved` must not count. +- A newer `review_requested` replaces earlier request evidence even if there was an old `review_started` in the same file. +- A latest `review_requested` without matching current-cycle `review_started` must be `review_pickup_required`. +- A legacy kanban reviewer can still create a review item, but should not get Phase 2 bypass unless there is a concrete `reviewRequestEventId`. +- A `review_started` by a different member is not a normal pickup case. Do not nudge the requested reviewer blindly; surface a diagnostic or lead escalation. +- A `review_started` with missing actor should not be treated as proof for Phase 2 bypass. It can suppress member spam, but it needs diagnostics because timer attribution may be impossible. +- If an anomalous `review_started` is followed by a valid current-cycle `review_started` by the requested reviewer, the valid start wins for obligation. Keep diagnostics for observability, but do not keep the task in pickup-required state. + +--- + +## 7. Agenda Item Shape + +For a requested-only review: + +```json +{ + "kind": "review", + "priority": "review_requested", + "reason": "current_cycle_review_assigned", + "evidence": { + "status": "completed", + "owner": "jack", + "reviewer": "alice", + "reviewState": "review", + "reviewObligation": "review_pickup_required", + "canBypassPhase2": true, + "reviewCycleId": "420d47fb-be29-40ab-8d2e-c2e4fad63961", + "reviewRequestEventId": "420d47fb-be29-40ab-8d2e-c2e4fad63961", + "reviewRequestedAt": "2026-05-09T08:05:28.361Z", + "historyEventIds": ["420d47fb-be29-40ab-8d2e-c2e4fad63961"] + } +} +``` + +For an already started review: + +```json +{ + "kind": "review", + "priority": "review_requested", + "reason": "current_cycle_review_assigned", + "evidence": { + "status": "completed", + "owner": "jack", + "reviewer": "alice", + "reviewState": "review", + "reviewObligation": "review_in_progress", + "canBypassPhase2": false, + "reviewCycleId": "420d47fb-be29-40ab-8d2e-c2e4fad63961", + "reviewRequestEventId": "420d47fb-be29-40ab-8d2e-c2e4fad63961", + "reviewRequestedAt": "2026-05-09T08:05:28.361Z", + "reviewStartedEventId": "abc-start", + "reviewStartedAt": "2026-05-09T08:06:10.000Z", + "reviewStartedBy": "alice", + "historyEventIds": ["420d47fb-be29-40ab-8d2e-c2e4fad63961", "abc-start"] + } +} +``` + +--- + +## 8. Nudge Activation Policy + +### 8.1 Current behavior + +Current activation allows: + +- all providers when `phase2Readiness.state === 'shadow_ready'`; +- OpenCode targeted candidates during `collecting_shadow_data`; +- no Anthropic/Codex/Gemini bypass while collecting. + +### 8.2 Desired review-specific bypass + +Add a narrow condition: + +```ts +function isReviewPickupRequired(status: MemberWorkSyncStatus): boolean { + return ( + status.state === 'needs_sync' && + status.shadow?.wouldNudge === true && + status.agenda.items.length > 0 && + status.agenda.items.every( + (item) => + item.kind === 'review' && + item.evidence.reviewObligation === 'review_pickup_required' && + Boolean(item.evidence.reviewRequestEventId) && + item.evidence.canBypassPhase2 === true + ) + ); +} +``` + +Then activation can allow planning: + +```ts +if (hasBlockingMetrics(input.metrics)) { + return { active: false, reason: 'blocking_metrics' }; +} + +if (isReviewPickupRequired(input.status)) { + return { active: true, reason: 'review_pickup_required' }; +} +``` + +Important: keep `blocking_metrics` before review bypass. If the team has unsafe nudge rates or fingerprint churn, do not bypass. + +But activation is not enough. Dispatch must also verify delivery capability: + +```ts +function hasReviewPickupDeliveryCapability(status: MemberWorkSyncStatus): boolean { + if (status.providerId === 'opencode') { + return true; // existing runtime wake + } + + return status.deliveryCapabilities?.memberWorkSyncNudgeWake === true; +} + +if (isReviewPickupRequired(status) && !hasReviewPickupDeliveryCapability(status)) { + return { active: false, reason: 'review_pickup_delivery_unavailable' }; +} +``` + +Without this gate, Anthropic can pass activation but still only get a passive inbox row. That is not a reliable repair. + +Use two different failure classes: + +```text +delivery capability absent = do not create/dispatch member outbox, audit and lead-escalate +provider delivery temporarily failed = retryable dispatch failure with nextAttemptAt +fire-and-forget wake scheduled = not enough to mark prompt_accepted +``` + +This distinction matters because a provider that has no implementation should not create an infinite retry loop, while an active provider with a transient wake error should retry. + +Suggested type change: + +```ts +export type MemberWorkSyncNudgeActivationReason = + | 'shadow_ready' + | 'opencode_targeted_shadow_collecting' + | 'review_pickup_required' + | 'review_pickup_delivery_unavailable' + | 'status_not_nudgeable' + | 'blocking_metrics' + | 'phase2_not_ready'; +``` + +If delivery capability is hard to expose through `MemberWorkSyncStatus`, keep it in dispatcher deps instead: + +```ts +nudgeDeliveryWake.canWake?.({ + teamName, + memberName, + providerId, + messageKind: 'member_work_sync_nudge', + workSyncIntent: 'review_pickup', +}); +``` + +The important part is the product invariant, not the exact API shape: + +```text +review pickup bypass may create a prompt only when the app can wake that member path +``` + +--- + +## 9. Nudge Payload + +### 9.1 Current payload is too generic + +Current text: + +```text +Work sync check: you have current actionable work assigned. +Required sync action: call member_work_sync_status... +Then call member_work_sync_report... +``` + +For review pickup, this is not enough. It can produce a valid `still_working` lease without `review_start`, which would hide the stuck review for up to 15 minutes. + +### 9.2 Review-specific payload + +If every item is `review_pickup_required`, build a different payload: + +```ts +function buildReviewPickupNudgePayload(status: MemberWorkSyncStatus): MemberWorkSyncNudgePayload { + const reviewItems = status.agenda.items.filter( + (item) => item.kind === 'review' && item.evidence.reviewObligation === 'review_pickup_required' + ); + + const taskIds = reviewItems.map((item) => item.taskId); + const taskList = reviewItems + .map((item) => `${item.displayId ?? item.taskId.slice(0, 8)} ${item.subject}`) + .join('; '); + + return { + from: 'system', + to: status.memberName, + messageKind: 'member_work_sync_nudge', + workSyncIntent: 'review_pickup', + workSyncIntentKey: buildReviewPickupIntentKey(reviewItems), + workSyncReviewRequestEventIds: reviewItems.map((item) => item.evidence.reviewRequestEventId), + source: 'member-work-sync', + actionMode: 'do', + taskRefs: reviewItems.map((item) => ({ + teamName: status.teamName, + taskId: item.taskId, + displayId: item.displayId ?? item.taskId.slice(0, 8), + })), + text: [ + 'Review pickup check: you have a current review request that is still waiting for review_start.', + `Current review agenda: ${taskList}.`, + `First call task_get for the task. If it is still in review for member "${status.memberName}", call review_start now.`, + `After review_start, either approve with review_approve or request fixes with review_request_changes.`, + 'Do not treat this as a duplicate only because you reviewed an earlier cycle. A later review request starts a new review cycle.', + `If you are blocked from reviewing, add or request concrete blocker evidence on the task before reporting blocked.`, + `member_work_sync_report may be used only to lease the sync state while you continue. It does not start or finish the review.`, + taskIds.length + ? `When reporting, include taskIds: ${taskIds.map((id) => `"${id}"`).join(', ')}.` + : '', + `Do not use provider names, runtime names, or team names as memberName; use exactly "${status.memberName}".`, + 'Do not reply only with acknowledgement.', + ] + .filter(Boolean) + .join('\n'), + }; +} +``` + +Required payload contract change: + +```ts +export interface MemberWorkSyncNudgePayload { + from: 'system'; + to: string; + messageKind: 'member_work_sync_nudge'; + workSyncIntent: 'agenda_sync' | 'review_pickup'; + workSyncIntentKey?: string; + workSyncReviewRequestEventIds?: string[]; + source: 'member-work-sync'; + actionMode: 'do'; + text: string; + taskRefs: TaskRef[]; +} +``` + +The same metadata should survive through: + +```text +MemberWorkSyncNudgePayload +-> TeamInboxMemberWorkSyncNudgeSink +-> SendMessageRequest +-> InboxMessage +-> TeamInboxWriter +-> TeamInboxReader +-> OpenCode delivery ledger / runtime adapter where applicable +``` + +Do not use text parsing as the main way to detect review-pickup intent. Text can remain a fallback for legacy rows only. + +### 9.3 OpenCode wrapper also needs review-specific wording + +OpenCode currently wraps work-sync nudges with: + +```text +Concrete task progress or member_work_sync_report is sufficient response proof. +``` + +For review pickup, that wording is wrong. The wrapper should inspect the payload or new payload kind and say: + +```text +This delivered app message is a review pickup work-sync nudge. +Concrete proof is review_start, review_approve, review_request_changes, or a valid member_work_sync_report lease. +The lease prevents repeated sync nudges but does not start or finish review. +``` + +This matters because OpenCode delivery proof currently treats report proof as enough to mark prompt delivery complete. That is fine for delivery, but not enough to clear the work-sync obligation unless the agenda changes or a lease is still active. + +Use structured metadata first: + +```ts +const isReviewPickupNudge = + input.messageKind === 'member_work_sync_nudge' && input.workSyncIntent === 'review_pickup'; +``` + +Text marker fallback is acceptable only for already persisted legacy messages: + +```ts +const isLegacyReviewPickupNudge = input.text.includes('Review pickup check:'); +``` + +--- + +## 10. Report Semantics + +### 10.1 Keep `still_working` as a lease + +Do not reject `still_working` for review pickup by default. If an agent is actually about to review, a short lease is useful. The important constraint is: + +```text +still_working suppresses repeat nudge +still_working does not satisfy review pickup +``` + +Current `decideMemberWorkSyncStatus()` already implements this behavior by returning `still_working` only until `expiresAt`. + +### 10.2 Shorter default lease for review pickup + +Current default still-working lease is 15 minutes. For review pickup this is too long. + +Suggested rule: + +```ts +const DEFAULT_REVIEW_PICKUP_STILL_WORKING_LEASE_MS = 3 * 60 * 1000; +``` + +Option A: + +- pass agenda into `clampLeaseTtlMs`; +- if agenda has `review_pickup_required`, default to 3 minutes and max to 10 minutes. +- 🎯 9 🛡️ 9 🧠 4, roughly 30-70 LOC. + +Option B: + +- keep report validator unchanged; +- nudge text explicitly asks for smaller `leaseTtlMs`. +- 🎯 6 🛡️ 5 🧠 2, roughly 10-25 LOC. + +Recommendation: Option A. It is safer because the app controls the lease. + +### 10.3 `blocked` must still require board evidence + +Current report validator rejects blocked unless agenda has blocker evidence: + +```ts +if ( + input.request.state === 'blocked' && + !agendaHasBlockedEvidence(input.agenda, input.request.taskIds) +) { + return { + ok: false, + code: 'blocked_without_evidence', + message: 'Blocked report requires current blocker evidence in the task board.', + }; +} +``` + +For review pickup, this should remain strict. A reviewer saying "blocked" without a task comment or blocker flag is not durable. + +Potential future improvement: + +```text +review_blocked_without_comment -> reject with message telling reviewer to add task comment first +``` + +### 10.4 Delivered outbox rows are terminal + +Current outbox semantics should be treated as: + +```text +pending / claimed / failed_retryable can be retried +delivered is terminal for that outbox id +``` + +That means after the short `still_working` lease expires, we must not rely on reviving the same delivered member nudge. If the same `reviewRequestEventId` is still `review_pickup_required`, the next action should be lead escalation or a new explicit escalation row, not another member poke under a churned agenda fingerprint. + +For review pickup, write the one-shot member marker only after the stronger delivery definition is met: + +```text +do not mark one-shot on outbox planned +do not mark one-shot on inbox inserted alone +do not mark one-shot on fire-and-forget wake scheduled +mark one-shot after prompt_accepted or response_proven +``` + +If wake fails after inbox insertion, the retry should be able to reuse the same message id and schedule wake again. + +--- + +## 11. One-Shot Member Nudge And Lead Escalation + +A key anti-spam requirement: + +```text +Do not keep poking the reviewer forever. +``` + +Recommended behavior: + +1. First failure after turn-settled: + - work-sync computes `needs_sync`; + - obligation is `review_pickup_required`; + - one member nudge is delivered for that review request cycle. + +2. If reviewer calls `review_start`, `review_approve`, or `review_request_changes`: + - agenda changes; + - outbox item is superseded or no longer matches; + - no more pickup nudges. + +3. If reviewer reports `still_working`: + - status becomes `still_working`; + - pending nudge is superseded; + - after short lease expires, work-sync can re-enter `needs_sync`. + +4. If after one delivered member nudge the same review cycle remains `review_pickup_required`: + - do not deliver another member nudge immediately; + - create a lead-facing escalation or diagnostic. + +Possible escalation payload: + +```text +Review pickup still pending after member correction. + +Task #7142f765 is still in review for alice, but no review_start, review_approve, or review_request_changes was recorded after the current review request. + +The member already received one review pickup correction for this cycle. Consider reassigning reviewer or sending a direct instruction. +``` + +Implementation options: + +- Minimal: record `review_pickup_member_nudge_delivered` in audit and rely on existing rate limit - 🎯 5 🛡️ 4 🧠 2, 20-40 LOC. This is not enough for reliable production because fingerprint churn can create repeated member nudges. +- Better: add outbox terminal reason or sidecar marker keyed by `(team, member, reviewRequestEventId)` - 🎯 8 🛡️ 8 🧠 5, 80-150 LOC. +- Best: add the sidecar marker plus a lead notification outbox path for ignored review pickup obligations - 🎯 9 🛡️ 9 🧠 7, 160-260 LOC. + +Recommended first pass: + +```text +one member nudge per reviewRequestEventId +lead notification after lease expiry or next turn-settled if obligation persists +``` + +This avoids repeated member spam while making the stuck state visible. + +Do not make lead escalation optional if the goal is to prevent future silent stuck reviews. Without escalation, the system avoids spam but can still leave the task invisible after the first ignored correction. + +--- + +## 12. Edge Cases + +### 12.1 Reviewer already started review in this cycle + +History: + +```text +review_requested alice +review_started alice +``` + +Expected: + +- obligation is `review_in_progress`; +- no review pickup bypass; +- timer can show reviewing; +- stall monitor handles no-progress-after-start cases. + +### 12.2 Reviewer approved without explicit `review_start` + +History: + +```text +review_requested alice +review_approved alice +``` + +Expected: + +- no review agenda item; +- any pending pickup nudge is superseded during dispatch revalidation; +- do not create a pickup nudge after approval. + +### 12.3 Reviewer requested changes without explicit `review_start` + +History: + +```text +review_requested alice +review_changes_requested alice +status_changed completed -> pending +``` + +Expected: + +- review obligation gone; +- owner gets needs-fix work item; +- no pickup nudge. + +### 12.4 Task returned to work after review + +History: + +```text +review_requested alice +review_started alice +status_changed completed -> in_progress +status_changed in_progress -> completed +review_requested alice +``` + +Expected: + +- old `review_started` is not part of current cycle; +- current cycle is requested-only; +- pickup obligation exists for latest request only. + +This is the exact live Alice shape. + +### 12.5 Repeated review request in same cycle + +History: + +```text +review_requested alice +review_requested alice +``` + +Expected: + +- latest request should become the current request event; +- older request should not keep the same fingerprint; +- one nudge per latest request event. + +Reason: a new request often means the owner added new context or asked for another pass. + +### 12.6 Reviewer changed before pickup + +History: + +```text +review_requested alice +review_requested bob +``` + +Expected: + +- Alice agenda loses the review item; +- Bob agenda gets `review_pickup_required`; +- any Alice pending nudge is superseded because fingerprint no longer matches. + +### 12.7 Kanban reviewer exists but no review_requested event + +State: + +```text +kanban.tasks[task].column = review +kanban.tasks[task].reviewer = alice +history has no review_requested +``` + +Expected: + +- agenda may include review item for backwards compatibility; +- do not allow Phase 2 bypass; +- reason should be legacy or diagnostics should mention missing request event. + +Reason: without a concrete request event, idempotency by review cycle is weak. + +### 12.8 Self-review + +State: + +```text +owner = alice +reviewer = alice +``` + +Current controller agenda treats self-review as lead oversight in some paths. Work-sync should avoid nudging Alice to review her own task unless the system explicitly allows self-review. + +Expected: + +- no review pickup bypass for self-review; +- lead-facing issue is safer. + +### 12.9 Reviewer inactive or removed + +Expected: + +- `activeMemberNames` validation prevents valid report; +- agenda source should not create a member agenda for removed member; +- task impact resolver should fall back to lead if reviewer missing/invalid; +- no member nudge. + +### 12.10 Team offline or stopping + +Expected: + +- `isTeamActive` false makes status `inactive`; +- outbox dispatch supersedes with `team_inactive`; +- no inbox insertion. + +### 12.11 Member busy + +Expected: + +- active or recent tool activity defers dispatch; +- retry happens after busy `retryAfterIso`; +- no interruption during active tool calls. + +### 12.12 Existing stall monitor alert + +Expected: + +- `TeamTaskStallJournalWorkSyncCooldown` prevents duplicate nudge if stall monitor recently alerted for same task; +- review pickup and stall monitor should not both spam. + +Important distinction: + +```text +review_pickup_required = reviewer has not started current review +started-review stall = reviewer started but stopped progressing +``` + +### 12.13 Agent reports `still_working` but does nothing + +Expected: + +- status becomes `still_working` for short lease; +- no repeated immediate nudge; +- after lease expires, obligation returns to `needs_sync`; +- if member already got one pickup nudge for this reviewRequestEventId, escalate to lead instead of repeatedly nudging the member. + +### 12.14 App crash between outbox creation and dispatch + +Expected: + +- pending outbox item remains durable; +- dispatch scheduler claims it after restart; +- revalidation prevents stale delivery. + +### 12.15 App crash after inbox write but before mark delivered + +Expected: + +- `insertIfAbsent` uses stable `messageId`; +- retry sees existing inbox row and does not duplicate message; +- outbox can mark delivered on retry. + +### 12.16 Agent reads nudge but app crashes before mark read + +Expected: + +- native inbox behavior may re-relay unread rows; +- stable `messageId` and relayed ids reduce duplicates during a run; +- for OpenCode, delivery ledger should handle response proof separately; +- for native, this is acceptable because nudge is idempotent and review tools are idempotent. + +### 12.17 OpenCode-specific delivery proof + +OpenCode wrapper currently treats work-sync report as enough delivery proof. For review pickup: + +- report is enough proof that the nudge was processed; +- report is not enough proof that review started; +- work-sync status remains `still_working` until lease expires unless task state changes. + +### 12.18 Non-OpenCode delivery outcome + +Production composition currently wires `nudgeDeliveryWake` only for OpenCode: + +```ts +nudgeDeliveryWake: { + schedule: (input) => { + if (input.providerId !== 'opencode') { + return; + } + teamProvisioningService.scheduleOpenCodeMemberInboxDeliveryWake(...); + }, +} +``` + +That is fine for generic Phase 2 because OpenCode was the targeted early-delivery candidate. For review pickup bypass, this becomes an implementation risk because the first target provider in the live incident is Anthropic. + +Delivery options: + +1. **Generic provider delivery outcome using a narrow, tested path** - 🎯 8 🛡️ 8 🧠 7, 180-300 LOC + Add a `canDeliver` / `deliver` capability for `member_work_sync_nudge` and route providers explicitly. OpenCode can reuse relay/ledger internals, but not just fire-and-forget scheduling. Native providers use a proven direct member inbox watcher result or `relayInboxFileToLiveRecipient` only if the service test proves it reaches the live member without the old lead-relay loop. + +2. **OpenCode-only rollout first** - 🎯 7 🛡️ 8 🧠 3, 40-80 LOC + Safe and fast, but it does not fix the live Anthropic/Alice incident. Use only as an incremental rollout, not as the final answer. + +3. **Re-enable old `relayMemberInboxMessages` for native** - 🎯 4 🛡️ 4 🧠 5, 60-140 LOC + Not recommended. The code comment says this path caused lead misrouting, duplicate messages, and relay loops. + +Required behavior: + +- work-sync may insert an inbox row for Anthropic; +- review pickup bypass may dispatch only when delivery capability is available; +- if capability is missing, do not create a member nudge outbox row; audit `review_pickup_delivery_unavailable` and surface lead diagnostic instead of pretending the member was corrected; +- if capability exists but provider delivery fails, keep the outbox retryable, because the persisted inbox row alone is not enough for review pickup; +- if provider delivery is fire-and-forget only, treat it as not capable for review pickup until there is a follow-up result path; +- do not assume inbox insertion alone is enough. + +Implementation gate: + +```text +Before enabling Anthropic review pickup bypass, add a service or live-smoke test proving that member_work_sync_nudge reaches an active Anthropic/native member path. +``` + +### 12.19 Native delivery marked read but not semantically processed + +Some delivery paths can verify "row written" or "prompt sent" without proving the member called a review tool. That is okay only if the task state remains the source of truth. + +Expected: + +- delivery proof can mark an outbox item delivered; +- delivery proof must not clear `review_pickup_required`; +- only `review_start`, `review_approve`, `review_request_changes`, or a short `member_work_sync_report` lease can change the next status decision; +- ignored delivered nudges escalate to lead. + +### 12.20 Multiple review items in one agenda + +If reviewer has multiple pending review pickups: + +- one nudge can list all current review pickups; +- idempotency by full agenda fingerprint still works; +- one-shot marker should be per `(member, reviewRequestEventId)`, not per whole agenda, otherwise adding a second review can accidentally unlock another nudge for the first. + +### 12.21 Fingerprint churn + +Changing subject, display id, or evidence can change the fingerprint. The bypass must still respect `blocking_metrics` when churn is high. + +Potential mitigation: + +- keep review pickup evidence minimal and stable; +- do not include non-essential timestamps beyond cycle timestamps; +- keep generatedAt out of fingerprint, as current code already does. + +### 12.22 Clock skew or malformed timestamps + +Expected: + +- event sort should preserve file order as fallback when timestamps are invalid or equal; +- `reviewCycleId` should prefer event id, not timestamp; +- malformed timestamps should not create duplicate cycles if event id exists. + +### 12.23 `review_started` actor missing or mismatched + +History: + +```text +review_requested alice +review_started actor missing +``` + +or: + +```text +review_requested alice +review_started bob +``` + +Expected: + +- do not use this as a normal review pickup bypass case; +- add diagnostics such as `review_started_actor_missing` or `review_started_by_different_member`; +- avoid repeatedly nudging Alice if evidence suggests the task lifecycle is corrupted; +- escalate to lead if the review remains stuck. + +### 12.24 Kanban/workflow mismatch in task impact routing + +State: + +```text +kanban column = review +task.reviewState missing or stale +kanban reviewer = alice +``` + +Expected: + +- full agenda and task-impact resolver both route Alice; +- resolver input must use kanban-aware `taskWorkflowColumn`, not raw `task.reviewState`; +- test this explicitly because otherwise the bug only appears in incremental updates. + +--- + +## 13. Implementation Plan + +### Phase 0 - Provider delivery capability gate + +Files: + +```text +src/features/member-work-sync/core/application/ports.ts +src/features/member-work-sync/core/application/MemberWorkSyncNudgeDispatcher.ts +src/features/member-work-sync/main/composition/createMemberWorkSyncFeature.ts +src/main/index.ts +src/main/services/team/TeamProvisioningService.ts +test/features/member-work-sync/core/application/MemberWorkSyncNudgeDispatcher.test.ts +test/main/services/team/TeamProvisioningServiceRelay.test.ts +``` + +Changes: + +- add a way for work-sync to ask whether a review-pickup nudge can wake this provider/member path; +- keep existing OpenCode wake behavior; +- add or prove a narrow native delivery outcome path before enabling Anthropic/Codex/Gemini review pickup bypass; +- audit `review_pickup_delivery_unavailable` when detection works but prompt delivery is not safe; +- classify capability failures as absent vs temporarily failed; +- do not re-enable the old lead relay path without new tests covering the bugs listed in `src/main/ipc/teams.ts`. + +Recommended API shape: + +```ts +export interface MemberWorkSyncNudgeDeliveryWakePort { + canWake?(input: { + teamName: string; + memberName: string; + providerId?: MemberWorkSyncProviderId | null; + messageKind: 'member_work_sync_nudge'; + workSyncIntent?: 'agenda_sync' | 'review_pickup'; + }): Promise | boolean; + + schedule(input: { + teamName: string; + memberName: string; + messageId: string; + providerId?: MemberWorkSyncProviderId | null; + reason: 'member_work_sync_nudge_inserted' | 'member_work_sync_nudge_existing'; + delayMs?: number; + workSyncIntent?: 'agenda_sync' | 'review_pickup'; + }): Promise | void; +} +``` + +Important: this existing wake-style API is not sufficient by itself for review pickup. Either extend it with a delivery outcome callback/result, or introduce a separate `MemberWorkSyncReviewPickupDeliveryPort` as described above. + +### Phase 1 - Strict review cycle domain + +Files: + +```text +src/features/member-work-sync/core/domain/currentReviewCycle.ts +test/features/member-work-sync/core/ActionableWorkAgenda.test.ts +agent-teams-controller/test/controller.test.js +test/main/services/team/stallMonitor/TeamTaskStallPolicy.test.ts +``` + +Changes: + +- add `resolveCurrentReviewCycle`; +- keep `resolveCurrentReviewOwner` as compatibility wrapper or replace its callers; +- model boundaries matching controller logic; +- expose `reviewObligation`, `canBypassPhase2`, and diagnostics; +- add shared lifecycle fixture tables and run equivalent cases against work-sync, controller agenda, and stall monitor where practical. + +Compatibility wrapper: + +```ts +export function resolveCurrentReviewOwner(input: { + reviewState?: string | null; + kanbanReviewer?: string | null; + historyEvents?: ReviewHistoryEventLike[]; +}): CurrentReviewOwner | null { + const cycle = resolveCurrentReviewCycle(input); + return cycle + ? { + reviewer: cycle.reviewer, + historyEventIds: cycle.historyEventIds, + } + : null; +} +``` + +Do not ship this phase if repeated `review_requested` still keeps an older `review_started`. That is the exact Alice failure. + +### Phase 2 - Agenda evidence and fingerprint + +Files: + +```text +src/features/member-work-sync/contracts/types.ts +src/features/member-work-sync/core/domain/ActionableWorkAgenda.ts +src/features/member-work-sync/core/domain/AgendaFingerprint.ts +src/features/member-work-sync/main/adapters/input/MemberWorkSyncTaskImpactResolver.ts +test/features/member-work-sync/main/adapters/input/MemberWorkSyncTaskImpactResolver.test.ts +``` + +Changes: + +- add review evidence fields; +- include fields in fingerprint through existing evidence copy; +- add diagnostics for legacy reviewer fallback. +- fix task-impact routing to pass kanban-aware `taskWorkflowColumn` into the review-cycle resolver. + +Example: + +```ts +const reviewCycle = isReviewWorkflow + ? resolveCurrentReviewCycle({ + reviewState: workflowColumn, + kanbanReviewer: input.kanbanReviewersByTaskId?.[task.id] ?? null, + historyEvents: task.historyEvents, + }) + : null; + +if (reviewCycle && sameMemberName(reviewCycle.reviewer, memberName)) { + items.push({ + ...base, + kind: 'review', + priority: 'review_requested', + reason: 'current_cycle_review_assigned', + evidence: { + status: task.status, + ...(owner ? { owner } : {}), + reviewer: memberName, + ...(task.reviewState ? { reviewState: task.reviewState } : {}), + reviewObligation: reviewCycle.obligation, + reviewCycleId: reviewCycle.reviewCycleId, + reviewRequestEventId: reviewCycle.requestEventId, + reviewRequestedAt: reviewCycle.requestedAt, + ...(reviewCycle.startedEventId ? { reviewStartedEventId: reviewCycle.startedEventId } : {}), + ...(reviewCycle.startedAt ? { reviewStartedAt: reviewCycle.startedAt } : {}), + ...(reviewCycle.startedBy ? { reviewStartedBy: reviewCycle.startedBy } : {}), + canBypassPhase2: reviewCycle.canBypassPhase2, + ...(reviewCycle.diagnostics.length > 0 ? { reviewDiagnostics: reviewCycle.diagnostics } : {}), + ...(reviewCycle.historyEventIds.length > 0 + ? { historyEventIds: reviewCycle.historyEventIds } + : {}), + }, + }); +} +``` + +### Phase 3 - Review-specific activation + +Files: + +```text +src/features/member-work-sync/core/application/MemberWorkSyncNudgeActivationPolicy.ts +src/features/member-work-sync/core/application/MemberWorkSyncNudgeOutboxPlanner.ts +test/features/member-work-sync/core/application/MemberWorkSyncNudgeActivationPolicy.test.ts +``` + +Changes: + +- add activation reason `review_pickup_required`; +- add activation/audit reason `review_pickup_delivery_unavailable`; +- allow bypass only if all nudgeable items are requested-only review pickups; +- keep blocking metrics guard first; +- require delivery capability before planning or dispatching non-OpenCode review pickup nudges; +- do not create a member outbox row for permanent capability absence. + +### Phase 4 - Review-specific payload and metadata + +Files: + +```text +src/features/member-work-sync/core/domain/MemberWorkSyncNudge.ts +src/features/member-work-sync/contracts/types.ts +src/features/member-work-sync/main/adapters/output/TeamInboxMemberWorkSyncNudgeSink.ts +src/shared/types/team.ts +src/main/services/team/TeamInboxWriter.ts +src/main/services/team/TeamInboxReader.ts +test/features/member-work-sync/core/MemberWorkSyncUseCases.test.ts +``` + +Changes: + +- detect review pickup agenda; +- build review-specific text; +- keep generic text for normal work-sync agendas; +- persist `workSyncIntent`, `workSyncIntentKey`, and review request ids through inbox read/write; +- make payload hash include metadata so payload conflicts are real. + +### Phase 5 - Runtime wrapper and delivery outcome update + +Files: + +```text +src/main/services/team/runtime/OpenCodeTeamRuntimeAdapter.ts +src/main/services/team/opencode/delivery/OpenCodePromptDeliveryLedger.ts +src/main/services/team/opencode/delivery/OpenCodePromptDeliveryRepairPolicy.ts +src/main/services/team/TeamProvisioningService.ts +test/main/services/team/OpenCodeTeamRuntimeAdapter.test.ts +test/main/services/team/TeamProvisioningServiceRelay.test.ts +``` + +Changes: + +- detect review pickup nudge by payload metadata, with text fallback only for legacy rows; +- explain `review_start` as required domain action; +- preserve metadata in OpenCode delivery ledger records; +- for review-pickup, record `prompt_accepted` or `response_proven` before marking the member outbox terminal delivered; +- fire-and-forget watchdog scheduling is not enough for terminal delivered; +- provider delivery failure after inbox insertion must become retryable, not terminal delivered; +- add native delivery outcome only if a focused test proves it reaches live member input without the disabled lead relay bugs. + +### Phase 6 - Short lease and one-shot marker + +Files: + +```text +src/features/member-work-sync/core/application/MemberWorkSyncReportValidator.ts +src/features/member-work-sync/core/application/MemberWorkSyncNudgeDispatcher.ts +src/features/member-work-sync/main/infrastructure/MemberWorkSyncOutboxStore.ts +test/features/member-work-sync/core/MemberWorkSyncUseCases.test.ts +``` + +Changes: + +- default `still_working` lease for review pickup to 3 minutes; +- cap requested review-pickup lease at 10 minutes; +- add one-shot member nudge marker keyed by `(teamName, memberName, reviewRequestEventId)`; +- write one-shot marker only after `prompt_accepted` or `response_proven`; +- never use full agenda fingerprint as the only one-shot key. + +```text +if delivered member nudge exists for same reviewRequestEventId: + do not deliver second member nudge + plan lead escalation +``` + +### Phase 7 - Queue, startup scan, and planning coverage + +Files: + +```text +src/features/member-work-sync/main/adapters/input/MemberWorkSyncTeamChangeRouter.ts +src/features/member-work-sync/main/infrastructure/MemberWorkSyncEventQueue.ts +src/features/member-work-sync/core/application/MemberWorkSyncDiagnosticsReader.ts +src/features/member-work-sync/core/application/MemberWorkSyncReconciler.ts +src/features/member-work-sync/core/application/MemberWorkSyncNudgeOutboxPlanner.ts +test/features/member-work-sync/main/adapters/input/MemberWorkSyncTeamChangeRouter.test.ts +test/features/member-work-sync/main/infrastructure/MemberWorkSyncEventQueue.test.ts +``` + +Why this phase exists: + +```text +outbox planning happens only during queue reconciliation +manual status reads do not plan nudges +startup scan is the recovery path after app restart +task_changed routing must include reviewer, owner, and lead fallback correctly +getStatus may return persisted status and must expose staleness +``` + +Required changes/tests: + +- preserve the existing queue-only planning model, but document it in diagnostics; +- after app restart, startup scan must enqueue active members and then dispatch due review-pickup nudges; +- if task impact resolver cannot parse the task id, fallback team-wide must still include the reviewer; +- if a queue item is dropped because team is inactive, startup/member-spawn/turn-settled must be enough to re-evaluate later; +- planner should preserve actual activation reason. Do not collapse `review_pickup_delivery_unavailable`, `blocking_metrics`, and `phase2_not_ready` into the same audit code. +- `getStatus` should not silently return an old pre-restart snapshot as if it were fresh; +- stale `getStatus` should enqueue queue reconciliation or show explicit diagnostics without directly planning an outbox row from the read path. + +### Phase 8 - Lead escalation + +Files: + +```text +src/features/member-work-sync/core/application/MemberWorkSyncNudgeDispatcher.ts +src/features/member-work-sync/main/adapters/output/TeamInboxMemberWorkSyncNudgeSink.ts +src/main/services/team/TeamDataService.ts +test/features/member-work-sync/core/application/MemberWorkSyncNudgeDispatcher.test.ts +``` + +Possible implementation: + +- new audit event `review_pickup_escalated`; +- use existing lead system notification path; +- only after one member nudge was delivered and the same `reviewRequestEventId` still has pickup obligation after lease expiry or next turn-settled. +- also escalate when delivery capability is absent, because there is no member prompt path to wait for. + +This phase is required for the non-spam reliability goal. Without it, the first ignored correction can still leave the review stuck silently. + +--- + +## 14. Test Plan + +### 14.1 Domain tests + +Add tests for `resolveCurrentReviewCycle`: + +```ts +it('returns pickup required for latest requested-only review cycle', () => {}); +it('does not reuse review_started before status reset', () => {}); +it('returns in-progress when review_started follows current review_requested', () => {}); +it('returns null after review_approved', () => {}); +it('returns null after review_changes_requested', () => {}); +it('falls back to kanban reviewer without phase bypass evidence', () => {}); +it('uses event id as stable cycle id when timestamps are duplicated', () => {}); +it('uses latest review_requested when two requests exist in one open window', () => {}); +it('does not phase-bypass when review_started actor is missing', () => {}); +it('diagnoses review_started by a different member', () => {}); +it('uses later valid review_started even after earlier malformed started event', () => {}); +``` + +### 14.2 Agenda tests + +Add or update `ActionableWorkAgenda.test.ts`: + +```ts +it('marks requested-only review agenda with review_pickup_required', () => {}); +it('marks started review agenda with review_in_progress', () => {}); +it('keeps old review_started out of reopened review cycle evidence', () => {}); +it('moves review obligation from alice to bob when reviewer changes', () => {}); +it('does not create owner work while task is in review workflow', () => {}); +it('sets canBypassPhase2 only for requested-only cycles with concrete event id', () => {}); +it('carries review diagnostics into evidence without making them nudgeable', () => {}); +``` + +### 14.2.1 Task impact routing tests + +Add tests for `MemberWorkSyncTaskImpactResolver`: + +```ts +it('routes kanban review reviewer when task.reviewState is stale', () => {}); +it('routes lead when review workflow has no resolvable reviewer', () => {}); +it('does not route old reviewer after a newer review_requested assigns another member', () => {}); +``` + +### 14.3 Activation tests + +Add tests: + +```ts +it('activates review pickup nudges while phase2 is collecting', () => {}); +it('does not activate review pickup when blocking metrics are present', () => {}); +it('does not dispatch review pickup when delivery capability is unavailable', () => {}); +it('does not activate started-review nudges through review pickup bypass', () => {}); +it('does not activate legacy kanban-only review through review pickup bypass', () => {}); +it('keeps existing OpenCode targeted behavior', () => {}); +``` + +### 14.4 Payload tests + +Add tests: + +```ts +it('builds review pickup nudge with review_start instructions', () => {}); +it('says member_work_sync_report does not start or finish review', () => {}); +it('keeps generic nudge for work agenda', () => {}); +it('persists workSyncIntent and reviewRequestEventIds through inbox write/read', () => {}); +it('uses structured review pickup metadata in payload hash', () => {}); +``` + +### 14.5 Use case tests + +Add tests in `MemberWorkSyncUseCases.test.ts`: + +```ts +it('plans OpenCode review pickup while phase2 is collecting', async () => {}); +it('skips Anthropic review pickup when no native delivery outcome is configured', async () => {}); +it('plans Anthropic review pickup only when native delivery outcome capability is configured', async () => {}); +it('supersedes review pickup nudge when review_start appears before dispatch', async () => {}); +it('does not dispatch review pickup nudge while member is busy', async () => {}); +it('keeps review pickup retryable when provider delivery fails after inbox insertion', async () => {}); +it('does not mark review pickup delivered only because fire-and-forget wake was scheduled', async () => {}); +it('marks review pickup delivered only after prompt_accepted or response_proven', async () => {}); +it('does not write one-shot marker when only inbox insertion succeeded', async () => {}); +it('does not write one-shot marker when only fire-and-forget wake was scheduled', async () => {}); +it('does not dispatch duplicate member nudge for same reviewRequestEventId after fingerprint churn', async () => {}); +it('escalates to lead when same reviewRequestEventId remains stuck after one delivered member nudge', async () => {}); +it('shortens still_working lease for review pickup agenda', async () => {}); +``` + +### 14.6 Feature integration tests + +Add tests in `createMemberWorkSyncFeature.test.ts`: + +```ts +it('audits delivery unavailable for Anthropic/native when delivery outcome capability is absent', async () => {}); +it('delivers review pickup through real outbox and provider wake when capability is configured', async () => {}); +it('retries provider delivery using the existing inbox row after restart before markDelivered', async () => {}); +it('revalidates and supersedes when the task is approved before dispatch', async () => {}); +it('keeps team inactive review pickup nudges undelivered', async () => {}); +``` + +### 14.7 OpenCode wrapper tests + +Add tests: + +```ts +it('sends review pickup work-sync nudges with review_start oriented instructions', async () => {}); +it('does not say report alone starts or finishes review', async () => {}); +it('detects review pickup by structured workSyncIntent before text fallback', async () => {}); +``` + +### 14.8 Cross-component lifecycle fixture tests + +Use one shared table of history shapes for: + +```text +work-sync currentReviewCycle +agent-teams-controller agenda reviewer resolution +stall monitor review window handling +renderer review timer fallback, if practical +``` + +Minimum fixture rows: + +```text +requested only +requested -> started +requested -> started -> approved +requested -> started -> in_progress -> completed -> requested +requested alice -> requested bob +requested alice -> started bob +requested alice -> started missing actor +duplicate timestamps with stable event order +``` + +### 14.9 Queue and restart tests + +Add tests: + +```ts +it('plans review pickup during queue reconciliation but not during manual status read', async () => {}); +it('startup scan after app restart plans stuck review pickup before UI relies on stale status', async () => {}); +it('getStatus marks old persisted review pickup status stale after restart', async () => {}); +it('getStatus enqueues refresh for stale persisted status without planning outbox directly', async () => {}); +it('falls back team-wide when task change detail cannot be parsed', async () => {}); +it('preserves review_pickup_delivery_unavailable in planner audit reason', async () => {}); +it('re-enqueues after inactive-team drop once member_spawned or turn_settled arrives', async () => {}); +``` + +--- + +## 15. Debugging Flow After Implementation + +When a review seems stuck: + +```bash +jq '.status | {state, providerId, diagnostics, shadow, agenda: .agenda.items}' \ + ~/.claude/teams//members//.member-work-sync/status.json +``` + +Expected stuck pickup: + +```json +{ + "state": "needs_sync", + "agenda": [ + { + "kind": "review", + "evidence": { + "reviewObligation": "review_pickup_required", + "reviewRequestEventId": "..." + } + } + ] +} +``` + +Then inspect audit: + +```bash +tail -n 80 ~/.claude/teams//members//.member-work-sync/journal.jsonl +``` + +Useful events: + +```text +agenda_loaded +decision_made +nudge_planned +nudge_delivered +nudge_skipped +nudge_superseded +member_busy +team_inactive +review_pickup_delivery_unavailable +review_pickup_wake_failed_retryable +review_pickup_member_nudge_delivered +review_pickup_escalated +``` + +Expected after successful pickup: + +```text +task history contains review_started after latest review_requested +work-sync agenda either changes to review_in_progress or remains still_working only while leased +member card shows reviewing timer from reviewIntervals or current review_started fallback +``` + +If status remains stale after app restart, inspect queue diagnostics: + +```bash +jq '.status.shadow, .status.diagnostics' \ + ~/.claude/teams//members//.member-work-sync/status.json +``` + +Expected startup recovery path: + +```text +startup_scan -> queue_reconciled -> nudge_planned -> provider delivery outcome +``` + +If the journal shows only manual `reconcile_started` from UI reads, that is not enough. Outbox planning should happen from queue reconciliation or an explicit queue-triggered repair. + +--- + +## 16. Rollout Notes + +Recommended rollout: + +1. Ship strict cycle resolver, agenda evidence, and task-impact routing fix first with bypass disabled. +2. Ship structured payload metadata and wrapper tests. +3. Enable OpenCode review pickup bypass first only through an outcome-returning relay/ledger path. +4. Enable Anthropic/Codex/Gemini only after delivery capability tests pass for that provider path. +5. Keep generic Phase 2 behavior unchanged. +6. Observe audit journals for: + - nudge volume; + - repeated fingerprints; + - `review_pickup_delivery_unavailable`; + - `review_pickup_escalated`; + - report rejection rate; + - stale fingerprint supersedes. + +Do not enable broad Anthropic/Codex/Gemini nudges as part of this. The bypass should be limited to: + +```text +review_pickup_required +``` + +Anthropic/native rollout gate: + +```text +provider review pickup bypass = disabled unless member_work_sync_nudge wake is proven for that provider path +``` + +--- + +## 17. Acceptance Criteria + +The implementation is correct when: + +- A latest requested-only review creates `review_pickup_required`. +- A current-cycle `review_started` changes obligation to `review_in_progress`. +- Old `review_started` events before status reset do not affect the latest cycle. +- Repeated `review_requested` uses the latest request and does not inherit old started evidence. +- Generic work-sync Phase 2 readiness remains unchanged. +- Requested-only review pickup can nudge OpenCode while phase2 is collecting. +- Requested-only review pickup can nudge Anthropic/native only when delivery outcome capability is implemented and tested. +- If native delivery capability is unavailable, the system audits/escalates instead of silently claiming repair. +- A review-pickup outbox row is not terminal `delivered` until provider outcome is `prompt_accepted` or `response_proven`. +- Fire-and-forget wake scheduling alone does not write `delivered` or one-shot marker. +- Provider delivery failure after inbox insertion is retryable and reuses the existing message id. +- Started reviews do not use pickup bypass. +- `still_working` leases review pickup briefly but does not clear the obligation forever. +- Dispatch revalidates and supersedes stale outbox rows. +- The member is not nudged while busy or team inactive. +- Repeated reconciles do not duplicate inbox messages for the same `reviewRequestEventId`, even if agenda fingerprint changes. +- A persisted one-shot marker prevents repeated member nudges for the same review request. +- One-shot marker is written only after the stronger review-pickup delivery definition is met. +- Lead escalation is emitted when a delivered review pickup nudge is ignored and the same review request remains stuck. +- OpenCode delivery text does not imply that report alone starts or finishes review. +- Structured `workSyncIntent` metadata survives inbox write/read and runtime delivery. +- Startup scan after restart can plan review pickup even when no new task event arrives. +- Stored `getStatus` snapshots expose staleness and enqueue reconciliation instead of silently looking fresh. +- Planner audit preserves exact skip reason, including delivery unavailable vs phase2 not ready. + +--- + +## 18. Final Recommendation + +Implement the review pickup path as a narrow obligation in work-sync, not as a separate watchdog. + +The safest version is: + +```text +strict current review cycle ++ review_pickup_required agenda evidence ++ task-impact routing uses kanban-aware workflow state ++ provider delivery capability gate ++ delivered means prompt_accepted/response_proven, not inbox inserted or watchdog scheduled ++ review-specific Phase 2 activation bypass ++ structured review-specific nudge metadata and text ++ short still_working lease ++ one member nudge per reviewRequestEventId ++ lead escalation if still stuck +``` + +This directly fixes the Alice class of failures only when the provider delivery gate and delivery outcome model are included. Without those, the plan would detect the stuck review but could still fail to prompt the live Anthropic member or falsely mark a scheduled wake as delivered. The full version keeps spam risk low because it stays inside the existing work-sync outbox, revalidation, busy-signal, cooldown, and rate-limit machinery, then escalates to lead instead of repeatedly poking the reviewer. diff --git a/src/features/runtime-provider-management/renderer/ui/RuntimeProviderManagementPanelView.tsx b/src/features/runtime-provider-management/renderer/ui/RuntimeProviderManagementPanelView.tsx index 2f094e24..28c15fff 100644 --- a/src/features/runtime-provider-management/renderer/ui/RuntimeProviderManagementPanelView.tsx +++ b/src/features/runtime-provider-management/renderer/ui/RuntimeProviderManagementPanelView.tsx @@ -1321,7 +1321,7 @@ function ProviderModelList({ actions.setModelQuery(event.target.value)} onClick={(event) => event.stopPropagation()} onKeyDown={(event) => event.stopPropagation()} diff --git a/src/main/index.ts b/src/main/index.ts index 9c168878..c716995d 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -964,14 +964,25 @@ function wireFileWatcherEvents(context: ServiceContext): void { if (detail === 'config.json') { TeamConfigReader.invalidateTeam(teamName); getTeamDataWorkerClient().invalidateTeamConfig(teamName); + teamDataService?.invalidateTeamRuntimeAdvisories(teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(teamName); } else if (detail === 'team.meta.json' || detail === 'members.meta.json') { TeamConfigReader.invalidateListTeamsCache(); getTeamDataWorkerClient().invalidateTeamConfig(teamName); + teamDataService?.invalidateTeamRuntimeAdvisories(teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(teamName); } } if (row.type === 'task') { TeamTaskReader.invalidateAllTasksCache(); + teamDataService?.invalidateTeamRuntimeAdvisories(teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(teamName); + } + + if (row.type === 'member-advisory') { + teamDataService?.invalidateTeamRuntimeAdvisories(teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(teamName); } memberWorkSyncFeature?.noteTeamChange(row as TeamChangeEvent); @@ -1279,7 +1290,8 @@ async function initializeServices(): Promise { teamDataService.setMemberRuntimeAdvisoryService(teamMemberRuntimeAdvisoryService); teamProvisioningService = new TeamProvisioningService(); teamProvisioningService.setMemberRuntimeAdvisoryInvalidator((teamName, memberName) => { - teamMemberRuntimeAdvisoryService.invalidateMemberAdvisory(teamName, memberName); + teamDataService?.invalidateMemberRuntimeAdvisory(teamName, memberName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(teamName, memberName); }); publishStartupStatus({ phase: 'runtime', @@ -1413,13 +1425,23 @@ async function initializeServices(): Promise { if (event.detail === 'config.json') { TeamConfigReader.invalidateTeam(event.teamName); getTeamDataWorkerClient().invalidateTeamConfig(event.teamName); + teamDataService?.invalidateTeamRuntimeAdvisories(event.teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(event.teamName); } else if (event.detail === 'team.meta.json' || event.detail === 'members.meta.json') { TeamConfigReader.invalidateListTeamsCache(); getTeamDataWorkerClient().invalidateTeamConfig(event.teamName); + teamDataService?.invalidateTeamRuntimeAdvisories(event.teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(event.teamName); } } if (event.type === 'task') { TeamTaskReader.invalidateAllTasksCache(); + teamDataService?.invalidateTeamRuntimeAdvisories(event.teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(event.teamName); + } + if (event.type === 'member-advisory') { + teamDataService?.invalidateTeamRuntimeAdvisories(event.teamName); + getTeamDataWorkerClient().invalidateMemberRuntimeAdvisory(event.teamName); } if ( teamDataService && diff --git a/src/main/ipc/review.ts b/src/main/ipc/review.ts index 108ecda9..fd840add 100644 --- a/src/main/ipc/review.ts +++ b/src/main/ipc/review.ts @@ -59,6 +59,7 @@ import type { BrowserWindow, IpcMain, IpcMainInvokeEvent } from 'electron'; const wrapReviewHandler = createIpcWrapper('IPC:review'); const logger = createLogger('IPC:review'); +const TEAM_TASK_CHANGE_SUMMARY_IPC_UNIQUE_REQUEST_LIMIT = 201; // --- Module-level state --- @@ -204,6 +205,37 @@ function sanitizeTaskChangeOptions(options?: unknown): TaskChangeRequestOptions }; } +function sanitizeTeamTaskChangeSummaryRequests(requests: unknown): TeamTaskChangeSummaryRequest[] { + if (!Array.isArray(requests)) { + return []; + } + + const sanitizedRequests: TeamTaskChangeSummaryRequest[] = []; + const seenTaskIds = new Set(); + for (const request of requests) { + if (sanitizedRequests.length >= TEAM_TASK_CHANGE_SUMMARY_IPC_UNIQUE_REQUEST_LIMIT) { + break; + } + if (!request || typeof request !== 'object') { + continue; + } + const raw = request as Record; + if (typeof raw.taskId !== 'string') { + continue; + } + const taskId = raw.taskId.trim(); + if (!taskId || seenTaskIds.has(taskId)) { + continue; + } + seenTaskIds.add(taskId); + sanitizedRequests.push({ + taskId, + options: sanitizeTaskChangeOptions(raw.options), + }); + } + return sanitizedRequests; +} + async function handleGetTaskChanges( _event: IpcMainInvokeEvent, teamName: string, @@ -222,23 +254,7 @@ async function handleGetTeamTaskChangeSummaries( teamName: string, requests: unknown ): Promise> { - const sanitizedRequests: TeamTaskChangeSummaryRequest[] = Array.isArray(requests) - ? requests - .map((request): TeamTaskChangeSummaryRequest | null => { - if (!request || typeof request !== 'object') { - return null; - } - const raw = request as Record; - if (typeof raw.taskId !== 'string' || raw.taskId.trim().length === 0) { - return null; - } - return { - taskId: raw.taskId.trim(), - options: sanitizeTaskChangeOptions(raw.options), - }; - }) - .filter((request): request is TeamTaskChangeSummaryRequest => request !== null) - : []; + const sanitizedRequests = sanitizeTeamTaskChangeSummaryRequests(requests); return wrapReviewHandler('getTeamTaskChangeSummaries', () => getChangeExtractor().getTeamTaskChangeSummaries(teamName, sanitizedRequests) diff --git a/src/main/ipc/teams.ts b/src/main/ipc/teams.ts index 7112857f..28f4d384 100644 --- a/src/main/ipc/teams.ts +++ b/src/main/ipc/teams.ts @@ -3056,8 +3056,10 @@ async function handleSendMessage( ledgerStatus: delivery.ledgerStatus, visibleReplyMessageId: delivery.visibleReplyMessageId, visibleReplyCorrelation: delivery.visibleReplyCorrelation, + queuedBehindMessageId: delivery.queuedBehindMessageId, reason: delivery.reason, diagnostics: delivery.diagnostics, + userVisibleImpact: provisioning.buildOpenCodeRuntimeDeliveryUserVisibleImpact(delivery), }; if ( !delivery.delivered && @@ -3078,6 +3080,11 @@ async function handleSendMessage( delivered: false, reason, diagnostics: [reason], + userVisibleImpact: provisioning.buildOpenCodeRuntimeDeliveryUserVisibleImpact({ + delivered: false, + reason, + diagnostics: [reason], + }), }; logger.warn( `OpenCode runtime delivery after sendMessage crashed for teammate "${memberName}": ${reason}` diff --git a/src/main/services/team/ChangeExtractorService.ts b/src/main/services/team/ChangeExtractorService.ts index 6515aa79..a0e2b362 100644 --- a/src/main/services/team/ChangeExtractorService.ts +++ b/src/main/services/team/ChangeExtractorService.ts @@ -336,9 +336,21 @@ export class ChangeExtractorService { teamName: string, requests: TeamTaskChangeSummaryRequest[] ): Promise { - const cappedRequests = requests - .filter((request) => typeof request.taskId === 'string' && request.taskId.trim().length > 0) - .slice(0, TEAM_TASK_CHANGE_SUMMARY_BATCH_LIMIT); + const inputRequests = Array.isArray(requests) ? requests : []; + const seenTaskIds = new Set(); + const uniqueRequests: TeamTaskChangeSummaryRequest[] = []; + for (const request of inputRequests) { + if (!request || typeof request !== 'object') { + continue; + } + const taskId = typeof request.taskId === 'string' ? request.taskId.trim() : ''; + if (!taskId || seenTaskIds.has(taskId)) { + continue; + } + seenTaskIds.add(taskId); + uniqueRequests.push({ ...request, taskId }); + } + const cappedRequests = uniqueRequests.slice(0, TEAM_TASK_CHANGE_SUMMARY_BATCH_LIMIT); const items: TeamTaskChangeSummaryItem[] = cappedRequests.map((request) => ({ taskId: request.taskId.trim(), changeSet: null, @@ -379,7 +391,7 @@ export class ChangeExtractorService { teamName, items, computedAt: new Date().toISOString(), - truncated: requests.length > cappedRequests.length || undefined, + truncated: uniqueRequests.length > cappedRequests.length || undefined, }; } diff --git a/src/main/services/team/TeamDataService.ts b/src/main/services/team/TeamDataService.ts index 07239571..e329ff4f 100644 --- a/src/main/services/team/TeamDataService.ts +++ b/src/main/services/team/TeamDataService.ts @@ -497,6 +497,14 @@ export class TeamDataService { this.memberRuntimeAdvisoryService = service; } + invalidateMemberRuntimeAdvisory(teamName: string, memberName: string): void { + this.memberRuntimeAdvisoryService.invalidateMemberAdvisory(teamName, memberName); + } + + invalidateTeamRuntimeAdvisories(teamName: string): void { + this.memberRuntimeAdvisoryService.invalidateTeamAdvisories(teamName); + } + private async getMemberRuntimeAdvisoriesForSnapshot( teamName: string, members: readonly Pick[] diff --git a/src/main/services/team/TeamDataWorkerClient.ts b/src/main/services/team/TeamDataWorkerClient.ts index c0a09973..582c783d 100644 --- a/src/main/services/team/TeamDataWorkerClient.ts +++ b/src/main/services/team/TeamDataWorkerClient.ts @@ -106,6 +106,11 @@ function summarizeWorkerRequest(request: TeamDataWorkerRequest): Record; - taskProgressTimes: ReadonlyMap; -} - -function includesAnyToken(value: string, tokens: readonly string[]): boolean { - return tokens.some((token) => value.includes(token)); -} - -function classifyRetryReason(message: string | undefined): MemberRuntimeAdvisory['reasonCode'] { - const normalized = message?.trim().toLowerCase(); - if (!normalized) { - return 'unknown'; - } - if (includesAnyToken(normalized, QUOTA_EXHAUSTED_TOKENS)) { - return 'quota_exhausted'; - } - if (includesAnyToken(normalized, RATE_LIMITED_TOKENS)) { - return 'rate_limited'; - } - if (includesAnyToken(normalized, AUTH_ERROR_TOKENS)) { - return 'auth_error'; - } - if (includesAnyToken(normalized, CODEX_NATIVE_TIMEOUT_TOKENS)) { - return 'codex_native_timeout'; - } - if (includesAnyToken(normalized, NETWORK_ERROR_TOKENS)) { - return 'network_error'; - } - if (includesAnyToken(normalized, PROVIDER_OVERLOADED_TOKENS)) { - return 'provider_overloaded'; - } - if (includesAnyToken(normalized, PROTOCOL_PROOF_MISSING_TOKENS)) { - return 'protocol_proof_missing'; - } - return 'backend_error'; -} - -function getRecordTimeMs(record: OpenCodePromptDeliveryLedgerRecord): number { - const candidates = [ - record.failedAt, - record.respondedAt, - record.lastObservedAt, - record.updatedAt, - record.createdAt, - ]; - for (const candidate of candidates) { - const time = Date.parse(candidate ?? ''); - if (Number.isFinite(time)) { - return time; - } - } - return 0; -} - -function isTerminalSuccessfulRecord(record: OpenCodePromptDeliveryLedgerRecord): boolean { - return ( - record.status === 'responded' && - Boolean(record.inboxReadCommittedAt || record.visibleReplyMessageId) - ); -} - -function isPotentialRuntimeDeliveryError(record: OpenCodePromptDeliveryLedgerRecord): boolean { - if (record.status === 'failed_terminal') { - return true; - } - return ( - record.status !== 'responded' && - (record.responseState === 'session_error' || - record.responseState === 'tool_error' || - record.responseState === 'permission_blocked' || - record.responseState === 'reconcile_failed') - ); -} - async function mapLimit( items: readonly T[], limit: number, @@ -218,8 +86,6 @@ async function mapLimit( } export class TeamMemberRuntimeAdvisoryService { - private readonly inboxReader = new TeamInboxReader(); - private readonly taskReader = new TeamTaskReader(); private readonly memberCache = new Map(); private readonly teamBatchCacheByTeam = new Map(); private readonly cacheGenerationByTeam = new Map(); @@ -229,7 +95,8 @@ export class TeamMemberRuntimeAdvisoryService { >(); constructor( - private readonly logsFinder: RuntimeAdvisoryLogsFinder = new TeamMemberLogsFinder() + private readonly logsFinder: RuntimeAdvisoryLogsFinder = new TeamMemberLogsFinder(), + private readonly proofReader = new OpenCodeRuntimeDeliveryProofReader() ) {} invalidateMemberAdvisory(teamName: string, memberName: string): void { @@ -249,6 +116,26 @@ export class TeamMemberRuntimeAdvisoryService { } } + invalidateTeamAdvisories(teamName: string): void { + const teamKey = this.normalizeToken(teamName); + if (!teamKey) { + return; + } + + this.cacheGenerationByTeam.set(teamKey, (this.cacheGenerationByTeam.get(teamKey) ?? 0) + 1); + this.teamBatchCacheByTeam.delete(teamKey); + for (const key of this.memberCache.keys()) { + if (key.startsWith(`${teamKey}::`)) { + this.memberCache.delete(key); + } + } + for (const key of this.inFlightBatchRequests.keys()) { + if (key.startsWith(`${teamKey}::`)) { + this.inFlightBatchRequests.delete(key); + } + } + } + async getMemberAdvisories( teamName: string, members: readonly Pick[] @@ -553,9 +440,9 @@ export class TeamMemberRuntimeAdvisoryService { for (const [memberKey, records] of recordsByMember) { if ( records.some((record) => { - const observedAt = getRecordTimeMs(record); + const observedAt = getOpenCodeRuntimeDeliveryRecordTimeMs(record); return ( - isPotentialRuntimeDeliveryError(record) && + isPotentialOpenCodeRuntimeDeliveryError(record) && Number.isFinite(observedAt) && now - observedAt <= OPENCODE_DELIVERY_ERROR_LOOKBACK_MS ); @@ -568,11 +455,21 @@ export class TeamMemberRuntimeAdvisoryService { return new Map(); } - const supersedingProofTimes = await this.readOpenCodeRuntimeDeliverySupersedingProofTimes( - teamName, - memberKeysWithRecentErrors, - recordsByMember - ); + const proofIndex = await this.proofReader + .readProofIndex({ + teamName, + activeMemberKeys: memberKeysWithRecentErrors, + recordsByMember, + }) + .catch((error) => { + logger.warn('OpenCode runtime delivery proof lookup failed; using empty proof index', { + teamName, + error: error instanceof Error ? error.message : String(error), + }); + return { + getSnapshot: () => ({}), + } satisfies OpenCodeRuntimeDeliveryProofIndex; + }); const result = new Map(); for (const [memberKey, records] of recordsByMember) { if (!memberKeysWithRecentErrors.has(memberKey)) { @@ -580,12 +477,7 @@ export class TeamMemberRuntimeAdvisoryService { } const originalName = activeMembersByKey.get(memberKey); const advisory = originalName - ? this.buildOpenCodeDeliveryAdvisoryFromRecords( - originalName, - records, - now, - supersedingProofTimes - ) + ? this.buildOpenCodeDeliveryAdvisoryFromRecords(originalName, records, now, proofIndex) : null; if (advisory && originalName) { result.set(originalName, advisory); @@ -606,16 +498,20 @@ export class TeamMemberRuntimeAdvisoryService { memberName: string, records: readonly OpenCodePromptDeliveryLedgerRecord[], now: number, - supersedingProofTimes: OpenCodeRuntimeDeliverySupersedingProofTimes + proofIndex: OpenCodeRuntimeDeliveryProofIndex ): MemberRuntimeAdvisory | null { const ordered = records .slice() - .sort((left, right) => getRecordTimeMs(right) - getRecordTimeMs(left)); - const latestSuccess = ordered.find(isTerminalSuccessfulRecord); + .sort( + (left, right) => + getOpenCodeRuntimeDeliveryRecordTimeMs(right) - + getOpenCodeRuntimeDeliveryRecordTimeMs(left) + ); + const latestSuccess = ordered.find(isTerminalSuccessfulOpenCodeDeliveryRecord); const latestError = ordered.find((record) => { - const observedAt = getRecordTimeMs(record); + const observedAt = getOpenCodeRuntimeDeliveryRecordTimeMs(record); return ( - isPotentialRuntimeDeliveryError(record) && + isPotentialOpenCodeRuntimeDeliveryError(record) && Number.isFinite(observedAt) && now - observedAt <= OPENCODE_DELIVERY_ERROR_LOOKBACK_MS ); @@ -623,212 +519,35 @@ export class TeamMemberRuntimeAdvisoryService { if (!latestError) { return null; } - if (latestSuccess && getRecordTimeMs(latestSuccess) > getRecordTimeMs(latestError)) { - return null; - } if ( - this.hasSupersedingProofForOpenCodeDeliveryRecord( - memberName, - latestError, - supersedingProofTimes - ) + latestSuccess && + getOpenCodeRuntimeDeliveryRecordTimeMs(latestSuccess) > + getOpenCodeRuntimeDeliveryRecordTimeMs(latestError) ) { return null; } - const message = selectOpenCodeRuntimeDeliveryReason(latestError); - if (!message) { + const decision = decideOpenCodeRuntimeDeliveryAdvisory({ + record: latestError, + proof: proofIndex.getSnapshot(memberName, latestError), + now, + }); + if (decision.action !== 'surface') { + return null; + } + + const message = decision.reason; + if (!message || !decision.observedAt) { return null; } - const observedAt = getRecordTimeMs(latestError); return { kind: 'api_error', - observedAt: new Date(Number.isFinite(observedAt) ? observedAt : now).toISOString(), - reasonCode: classifyRetryReason(message), + observedAt: decision.observedAt, + reasonCode: decision.reasonCode, message, }; } - private async readOpenCodeRuntimeDeliverySupersedingProofTimes( - teamName: string, - activeMemberKeys: ReadonlySet, - recordsByMember: ReadonlyMap - ): Promise { - const [visibleReplyTimes, taskProgressTimes] = await Promise.all([ - this.readVisibleOpenCodeRuntimeDeliveryReplyTimes(teamName, activeMemberKeys), - this.readTaskProgressProofTimes(teamName, activeMemberKeys, recordsByMember), - ]); - return { visibleReplyTimes, taskProgressTimes }; - } - - private async readVisibleOpenCodeRuntimeDeliveryReplyTimes( - teamName: string, - activeMemberKeys: ReadonlySet - ): Promise> { - const result = new Map(); - const inboxNames = await this.inboxReader.listInboxNames(teamName).catch(() => []); - await mapLimit(inboxNames, ADVISORY_FETCH_CONCURRENCY, async (inboxName) => { - const messages = await this.inboxReader.getMessagesFor(teamName, inboxName).catch(() => []); - for (const message of messages) { - if (message.source !== 'runtime_delivery' || !message.relayOfMessageId) { - continue; - } - const memberKey = this.normalizeToken(message.from); - if (activeMemberKeys.has(memberKey)) { - const observedAt = Date.parse(message.timestamp); - if (!Number.isFinite(observedAt)) { - continue; - } - const key = this.getOpenCodeRuntimeReplyKey(memberKey, message.relayOfMessageId); - result.set(key, Math.max(result.get(key) ?? 0, observedAt)); - } - } - }); - return result; - } - - private hasVisibleRuntimeReplyForOpenCodeDeliveryRecord( - memberName: string, - record: OpenCodePromptDeliveryLedgerRecord, - visibleRuntimeReplyTimes: ReadonlyMap - ): boolean { - const relayOfMessageId = record.inboxMessageId?.trim(); - if (!relayOfMessageId) { - return false; - } - const replyObservedAt = visibleRuntimeReplyTimes.get( - this.getOpenCodeRuntimeReplyKey(this.normalizeToken(memberName), relayOfMessageId) - ); - return typeof replyObservedAt === 'number' && replyObservedAt > getRecordTimeMs(record); - } - - private hasSupersedingProofForOpenCodeDeliveryRecord( - memberName: string, - record: OpenCodePromptDeliveryLedgerRecord, - proofTimes: OpenCodeRuntimeDeliverySupersedingProofTimes - ): boolean { - if ( - this.hasVisibleRuntimeReplyForOpenCodeDeliveryRecord( - memberName, - record, - proofTimes.visibleReplyTimes - ) - ) { - return true; - } - - return this.hasTaskProgressProofForOpenCodeDeliveryRecord( - memberName, - record, - proofTimes.taskProgressTimes - ); - } - - private async readTaskProgressProofTimes( - teamName: string, - activeMemberKeys: ReadonlySet, - recordsByMember: ReadonlyMap - ): Promise> { - const taskIdsByMember = new Map>(); - for (const [memberKey, records] of recordsByMember) { - if (!activeMemberKeys.has(memberKey)) { - continue; - } - for (const record of records) { - for (const taskRef of record.taskRefs) { - const taskId = taskRef.taskId?.trim(); - if (!taskId) { - continue; - } - const taskIds = taskIdsByMember.get(memberKey) ?? new Set(); - taskIds.add(taskId); - taskIdsByMember.set(memberKey, taskIds); - } - } - } - if (taskIdsByMember.size === 0) { - return new Map(); - } - - const tasks = await this.taskReader.getTasks(teamName).catch(() => []); - if (tasks.length === 0) { - return new Map(); - } - - const result = new Map(); - for (const task of tasks) { - const taskId = task.id?.trim(); - if (!taskId) { - continue; - } - for (const [memberKey, taskIds] of taskIdsByMember) { - if (!taskIds.has(taskId)) { - continue; - } - const proofAt = this.getLatestMemberTaskProgressTime(task, memberKey); - if (proofAt <= 0) { - continue; - } - const key = this.getOpenCodeTaskProgressProofKey(memberKey, taskId); - result.set(key, Math.max(result.get(key) ?? 0, proofAt)); - } - } - return result; - } - - private getLatestMemberTaskProgressTime(task: TeamTask, memberKey: string): number { - let latest = 0; - for (const comment of task.comments ?? []) { - if (this.normalizeToken(comment.author) !== memberKey) { - continue; - } - const createdAt = Date.parse(comment.createdAt); - if (Number.isFinite(createdAt)) { - latest = Math.max(latest, createdAt); - } - } - for (const event of task.historyEvents ?? []) { - if (this.normalizeToken(event.actor ?? '') !== memberKey) { - continue; - } - const timestamp = Date.parse(event.timestamp); - if (Number.isFinite(timestamp)) { - latest = Math.max(latest, timestamp); - } - } - return latest; - } - - private hasTaskProgressProofForOpenCodeDeliveryRecord( - memberName: string, - record: OpenCodePromptDeliveryLedgerRecord, - taskProgressTimes: ReadonlyMap - ): boolean { - const recordTime = getRecordTimeMs(record); - if (!Number.isFinite(recordTime) || recordTime <= 0 || record.taskRefs.length === 0) { - return false; - } - const memberKey = this.normalizeToken(memberName); - return record.taskRefs.some((taskRef) => { - const taskId = taskRef.taskId?.trim(); - if (!taskId) { - return false; - } - const proofAt = taskProgressTimes.get( - this.getOpenCodeTaskProgressProofKey(memberKey, taskId) - ); - return typeof proofAt === 'number' && proofAt > recordTime; - }); - } - - private getOpenCodeRuntimeReplyKey(memberKey: string, relayOfMessageId: string): string { - return `${memberKey}::${relayOfMessageId.trim()}`; - } - - private getOpenCodeTaskProgressProofKey(memberKey: string, taskId: string): string { - return `${memberKey}::task::${taskId.trim()}`; - } - private async findRecentMemberAdvisoriesFromBatchRefs( teamName: string, memberNames: readonly string[] @@ -982,7 +701,7 @@ export class TeamMemberRuntimeAdvisoryService { observedAt: new Date(observedAt).toISOString(), retryUntil: new Date(retryUntil).toISOString(), retryDelayMs: retryInMs, - reasonCode: classifyRetryReason(message), + reasonCode: classifyOpenCodeRuntimeDeliveryReasonCode(message), ...(message ? { message } : {}), }; } catch { @@ -1034,7 +753,7 @@ export class TeamMemberRuntimeAdvisoryService { return { kind: 'api_error', observedAt: new Date(observedAt).toISOString(), - reasonCode: classifyRetryReason(message || parsed.error), + reasonCode: classifyOpenCodeRuntimeDeliveryReasonCode(message || parsed.error), ...(message ? { message } : {}), ...(statusMatch ? { statusCode: Number(statusMatch[1]) } : {}), }; diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 2f7aa5de..8fa10120 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -199,9 +199,24 @@ import { type OpenCodeVisibleReplyProof, } from './opencode/delivery/OpenCodePromptDeliveryWatchdog'; import { - isActionRequiredOpenCodeRuntimeDeliveryReason, - selectOpenCodeRuntimeDeliveryReason, -} from './opencode/delivery/OpenCodeRuntimeDeliveryDiagnostics'; + classifyOpenCodeRuntimeDeliveryReasonCode, + decideOpenCodeRuntimeDeliveryAdvisory, + isDeferredGenericOpenCodeRuntimeDeliveryReason, + isPotentialOpenCodeRuntimeDeliveryError, + toOpenCodeRuntimeDeliveryUserVisibleImpact, + type OpenCodeRuntimeDeliveryAdvisoryDecision, +} from './opencode/delivery/OpenCodeRuntimeDeliveryAdvisoryPolicy'; +import { selectOpenCodeRuntimeDeliveryReason } from './opencode/delivery/OpenCodeRuntimeDeliveryDiagnostics'; +import { + getOpenCodeVisibleReplyInboxCandidates as resolveOpenCodeVisibleReplyInboxCandidates, + isOpenCodeLeadReplyRecipientAlias as isOpenCodeLeadReplyRecipientAliasValue, + isOpenCodeRecoveredVisibleReplyCandidate as isOpenCodeRecoveredVisibleReplyCandidateValue, + isOpenCodeVisibleReplyTimestampEligible as isOpenCodeVisibleReplyTimestampEligibleValue, + normalizeOpenCodeTaskRefsForComparison as normalizeOpenCodeTaskRefsForComparisonValue, + openCodeTaskRefKey as openCodeTaskRefKeyValue, + openCodeTaskRefsIncludeAll as openCodeTaskRefsIncludeAllValue, +} from './opencode/delivery/OpenCodeRuntimeDeliveryProofMatching'; +import { OpenCodeRuntimeDeliveryProofReader } from './opencode/delivery/OpenCodeRuntimeDeliveryProofReader'; import { createRuntimeDeliveryJournalStore } from './opencode/delivery/RuntimeDeliveryJournal'; import { type RuntimeDeliveryDestinationPort, @@ -483,6 +498,7 @@ import type { OpenCodeAppManagedBootstrapCandidate, OpenCodeBootstrapEvidenceSource, OpenCodeRuntimeDeliveryStatus, + OpenCodeRuntimeDeliveryUserVisibleImpact, PersistedTeamLaunchMemberState, PersistedTeamLaunchPhase, PersistedTeamLaunchSnapshot, @@ -5333,6 +5349,7 @@ interface OpenCodeMemberInboxDelivery { queuedBehindMessageId?: string; reason?: string; diagnostics?: string[]; + userVisibleImpact?: OpenCodeRuntimeDeliveryUserVisibleImpact; } type OpenCodeVisibleReplyCorrelation = NonNullable< @@ -5478,8 +5495,10 @@ export class TeamProvisioningService { Promise >(); private readonly openCodePromptDeliveryWatchdogTimers = new Map(); + private readonly openCodeRuntimeDeliveryAdvisoryReviewTimers = new Map(); private readonly openCodeRuntimeDeliveryAdvisoryEventSentAt = new Map(); private readonly openCodeRuntimeDeliveryLeadNoticeSentAt = new Map(); + private readonly openCodeRuntimeDeliveryProofReader = new OpenCodeRuntimeDeliveryProofReader(); private readonly openCodePromptDeliveryWatchdogQueue: { teamName: string; run: () => Promise; @@ -7306,12 +7325,7 @@ export class TeamProvisioningService { message: InboxMessage; ledgerRecord: OpenCodePromptDeliveryLedgerRecord; }): boolean { - const messageMs = Date.parse(input.message.timestamp); - const inboxMs = Date.parse(input.ledgerRecord.inboxTimestamp); - if (!Number.isFinite(messageMs) || !Number.isFinite(inboxMs)) { - return true; - } - return messageMs + 5_000 >= inboxMs; + return isOpenCodeVisibleReplyTimestampEligibleValue(input); } private isOpenCodeRecoveredVisibleReplyCandidate(input: { @@ -7320,33 +7334,7 @@ export class TeamProvisioningService { from: string; requireTaskRefs: boolean; }): boolean { - const expectedFrom = input.from.trim().toLowerCase(); - if (!expectedFrom || input.message.from.trim().toLowerCase() !== expectedFrom) { - return false; - } - if (input.message.source !== undefined && input.message.source !== 'runtime_delivery') { - return false; - } - if ( - input.requireTaskRefs && - !this.openCodeTaskRefsIncludeAll(input.message.taskRefs, input.ledgerRecord.taskRefs) - ) { - return false; - } - if ( - !this.isOpenCodeVisibleReplyTimestampEligible({ - message: input.message, - ledgerRecord: input.ledgerRecord, - }) - ) { - return false; - } - return isOpenCodeVisibleReplySemanticallySufficient({ - actionMode: input.ledgerRecord.actionMode, - taskRefs: input.ledgerRecord.taskRefs, - text: input.message.text, - summary: input.message.summary, - }).sufficient; + return isOpenCodeRecoveredVisibleReplyCandidateValue(input); } private async correlateOpenCodeRecoveredVisibleReply(input: { @@ -7536,88 +7524,37 @@ export class TeamProvisioningService { replyRecipient?: string | null; includeUserFallbackForLeadRecipient?: boolean; }): Promise { - const explicitRecipient = input.replyRecipient?.trim() || 'user'; - const candidates = [explicitRecipient]; const configuredLeadName = await this.readConfigForObservation(input.teamName) .then( (config) => config?.members?.find((member) => isLeadMember(member))?.name?.trim() || null ) .catch(() => null); - const isConfiguredLeadRecipient = - Boolean(configuredLeadName) && - configuredLeadName?.toLowerCase() === explicitRecipient.toLowerCase(); - - if (this.isOpenCodeLeadReplyRecipientAlias(explicitRecipient) || isConfiguredLeadRecipient) { - if (configuredLeadName) { - candidates.push(configuredLeadName); - } - candidates.push('lead'); - candidates.push('team-lead'); - if (input.includeUserFallbackForLeadRecipient) { - candidates.push('user'); - } - } - return candidates - .filter((value): value is string => Boolean(value && value.trim())) - .filter( - (value, index, list) => - list.findIndex((item) => item.toLowerCase() === value.toLowerCase()) === index - ); + return resolveOpenCodeVisibleReplyInboxCandidates({ + replyRecipient: input.replyRecipient, + configuredLeadName, + includeUserFallbackForLeadRecipient: input.includeUserFallbackForLeadRecipient, + }); } private isOpenCodeLeadReplyRecipientAlias(value: string): boolean { - const normalized = value - .trim() - .toLowerCase() - .replace(/[\s_]+/g, '-'); - return ( - normalized === 'lead' || - normalized === 'team-lead' || - normalized === 'teamlead' || - normalized === 'team-leader' - ); + return isOpenCodeLeadReplyRecipientAliasValue(value); } private openCodeTaskRefsIncludeAll( actual: readonly TaskRef[] | undefined, expected: readonly TaskRef[] | undefined ): boolean { - const normalizedExpected = this.normalizeOpenCodeTaskRefsForComparison(expected); - if (normalizedExpected.length === 0) { - return true; - } - const actualKeys = new Set( - this.normalizeOpenCodeTaskRefsForComparison(actual).map((taskRef) => - this.openCodeTaskRefKey(taskRef) - ) - ); - return normalizedExpected.every((taskRef) => actualKeys.has(this.openCodeTaskRefKey(taskRef))); + return openCodeTaskRefsIncludeAllValue(actual, expected); } private normalizeOpenCodeTaskRefsForComparison( taskRefs: readonly TaskRef[] | undefined ): TaskRef[] { - if (!Array.isArray(taskRefs)) { - return []; - } - const normalized: TaskRef[] = []; - for (const rawTaskRef of taskRefs as readonly unknown[]) { - if (!rawTaskRef || typeof rawTaskRef !== 'object') { - continue; - } - const taskRef = rawTaskRef as Record; - const teamName = typeof taskRef.teamName === 'string' ? taskRef.teamName.trim() : ''; - const taskId = typeof taskRef.taskId === 'string' ? taskRef.taskId.trim() : ''; - const displayId = typeof taskRef.displayId === 'string' ? taskRef.displayId.trim() : ''; - if (teamName && taskId && displayId) { - normalized.push({ teamName, taskId, displayId }); - } - } - return normalized; + return normalizeOpenCodeTaskRefsForComparisonValue(taskRefs); } private openCodeTaskRefKey(taskRef: TaskRef): string { - return `${taskRef.teamName.trim()}\u0000${taskRef.taskId.trim()}\u0000${taskRef.displayId.trim()}`; + return openCodeTaskRefKeyValue(taskRef); } private async ensureOpenCodeVisibleReplyTaskRefs(input: { @@ -8446,59 +8383,84 @@ export class TeamProvisioningService { const shouldNotifyTerminalFailure = event === 'opencode_prompt_delivery_terminal_failure' && record.status === 'failed_terminal'; const shouldNotifyActionRequiredRetry = - !shouldNotifyTerminalFailure && - this.shouldNotifyOpenCodeRuntimeDeliveryBeforeTerminal(record); + !shouldNotifyTerminalFailure && isPotentialOpenCodeRuntimeDeliveryError(record); if (shouldNotifyTerminalFailure || shouldNotifyActionRequiredRetry) { - void this.fireOpenCodeRuntimeDeliveryErrorNotification(record).catch((error) => { + void this.handleOpenCodeRuntimeDeliveryUserFacingSideEffects(record).catch((error) => { logger.warn( - `[${record.teamName}] Failed to fire OpenCode runtime delivery error notification for ${record.memberName}: ${getErrorMessage(error)}` + `[${record.teamName}] Failed to handle OpenCode runtime delivery advisory side effects for ${record.memberName}: ${getErrorMessage(error)}` ); }); + } + } + + private async handleOpenCodeRuntimeDeliveryUserFacingSideEffects( + record: OpenCodePromptDeliveryLedgerRecord + ): Promise { + const { record: latestRecord, decision } = + await this.decideOpenCodeRuntimeDeliveryUserFacingAdvisory(record); + if (decision.action === 'defer') { + this.emitOpenCodeRuntimeDeliveryAdvisoryEvent(latestRecord, decision); + this.scheduleOpenCodeRuntimeDeliveryAdvisoryReview(latestRecord, decision); return; } - if (this.shouldSurfaceOpenCodeRuntimeDeliveryAdvisory(record)) { - this.emitOpenCodeRuntimeDeliveryAdvisoryEvent(record); + if (decision.action === 'suppress') { + this.emitOpenCodeRuntimeDeliveryAdvisoryEvent(latestRecord, decision); + return; } + + this.emitOpenCodeRuntimeDeliveryAdvisoryEvent(latestRecord, decision); + if (decision.severity !== 'error') { + return; + } + + await this.fireOpenCodeRuntimeDeliveryErrorNotification(latestRecord, decision); } - private shouldSurfaceOpenCodeRuntimeDeliveryAdvisory( + private async decideOpenCodeRuntimeDeliveryUserFacingAdvisory( record: OpenCodePromptDeliveryLedgerRecord - ): boolean { - if (!selectOpenCodeRuntimeDeliveryReason(record)) { - return false; + ): Promise<{ + record: OpenCodePromptDeliveryLedgerRecord; + decision: OpenCodeRuntimeDeliveryAdvisoryDecision; + }> { + const memberKey = record.memberName.trim().toLowerCase(); + let recordsForMember: OpenCodePromptDeliveryLedgerRecord[] = [record]; + try { + const laneRecords = await this.createOpenCodePromptDeliveryLedger( + record.teamName, + record.laneId + ).list(); + recordsForMember = laneRecords.filter( + (candidate) => candidate.memberName.trim().toLowerCase() === memberKey + ); + } catch { + recordsForMember = [record]; } - if (record.status === 'failed_terminal') { - return true; - } - if (record.status === 'responded') { - return false; - } - return ( - record.responseState === 'session_error' || - record.responseState === 'tool_error' || - record.responseState === 'permission_blocked' || - record.responseState === 'reconcile_failed' - ); - } - - private shouldNotifyOpenCodeRuntimeDeliveryBeforeTerminal( - record: OpenCodePromptDeliveryLedgerRecord - ): boolean { - if (!this.shouldSurfaceOpenCodeRuntimeDeliveryAdvisory(record)) { - return false; - } - if (record.status === 'failed_terminal') { - return false; - } - return isActionRequiredOpenCodeRuntimeDeliveryReason( - selectOpenCodeRuntimeDeliveryReason(record) - ); + const latestRecord = recordsForMember.find((candidate) => candidate.id === record.id) ?? record; + const recordsByMember = new Map([ + [memberKey, recordsForMember.length > 0 ? recordsForMember : [latestRecord]], + ]); + const activeMemberKeys = new Set([memberKey]); + const proofIndex = await this.openCodeRuntimeDeliveryProofReader + .readProofIndex({ + teamName: latestRecord.teamName, + activeMemberKeys, + recordsByMember, + }) + .catch(() => null); + return { + record: latestRecord, + decision: decideOpenCodeRuntimeDeliveryAdvisory({ + record: latestRecord, + proof: proofIndex?.getSnapshot(latestRecord.memberName, latestRecord), + }), + }; } private async fireOpenCodeRuntimeDeliveryErrorNotification( - record: OpenCodePromptDeliveryLedgerRecord + record: OpenCodePromptDeliveryLedgerRecord, + decision: OpenCodeRuntimeDeliveryAdvisoryDecision ): Promise { - const reason = this.selectOpenCodeRuntimeDeliveryNotificationReason(record); + const reason = decision.reason; if (!reason) { return; } @@ -8536,8 +8498,6 @@ export class TeamProvisioningService { ); } - this.emitOpenCodeRuntimeDeliveryAdvisoryEvent(record); - await this.notifyLeadAboutOpenCodeRuntimeDeliveryError({ record, reason, @@ -8546,7 +8506,8 @@ export class TeamProvisioningService { } private emitOpenCodeRuntimeDeliveryAdvisoryEvent( - record: OpenCodePromptDeliveryLedgerRecord + record: OpenCodePromptDeliveryLedgerRecord, + decision?: OpenCodeRuntimeDeliveryAdvisoryDecision ): void { try { this.memberRuntimeAdvisoryInvalidator?.(record.teamName, record.memberName); @@ -8556,7 +8517,7 @@ export class TeamProvisioningService { ); } - const reasonKey = this.getOpenCodeRuntimeDeliveryAdvisoryReasonKey(record); + const reasonKey = this.getOpenCodeRuntimeDeliveryAdvisoryReasonKey(record, decision); const eventKey = `opencode_runtime_delivery_error:${record.teamName}:${record.memberName}:${record.id}:${reasonKey}`; const now = Date.now(); this.pruneOpenCodeRuntimeDeliveryAdvisoryEventDedupe(now); @@ -8623,10 +8584,14 @@ export class TeamProvisioningService { } private getOpenCodeRuntimeDeliveryAdvisoryReasonKey( - record: OpenCodePromptDeliveryLedgerRecord + record: OpenCodePromptDeliveryLedgerRecord, + decision?: OpenCodeRuntimeDeliveryAdvisoryDecision ): string { const reason = - selectOpenCodeRuntimeDeliveryReason(record) ?? record.responseState ?? record.status; + decision?.reason ?? + selectOpenCodeRuntimeDeliveryReason(record) ?? + record.responseState ?? + record.status; const normalized = reason .toLowerCase() .replace(/https?:\/\/\S+/g, '') @@ -8636,6 +8601,31 @@ export class TeamProvisioningService { return normalized || 'unknown'; } + private scheduleOpenCodeRuntimeDeliveryAdvisoryReview( + record: OpenCodePromptDeliveryLedgerRecord, + decision: OpenCodeRuntimeDeliveryAdvisoryDecision + ): void { + const reviewAt = Date.parse(decision.nextReviewAt ?? ''); + if (!Number.isFinite(reviewAt)) { + return; + } + const delayMs = Math.max(250, reviewAt - Date.now()); + const timerKey = `${record.teamName}:${record.laneId}:${record.id}`; + const existing = this.openCodeRuntimeDeliveryAdvisoryReviewTimers.get(timerKey); + if (existing) { + clearTimeout(existing); + } + const timer = setTimeout(() => { + this.openCodeRuntimeDeliveryAdvisoryReviewTimers.delete(timerKey); + void this.handleOpenCodeRuntimeDeliveryUserFacingSideEffects(record).catch((error) => { + logger.warn( + `[${record.teamName}] Failed to refresh deferred OpenCode runtime delivery advisory for ${record.memberName}: ${getErrorMessage(error)}` + ); + }); + }, delayMs); + this.openCodeRuntimeDeliveryAdvisoryReviewTimers.set(timerKey, timer); + } + private async notifyLeadAboutOpenCodeRuntimeDeliveryError(input: { record: OpenCodePromptDeliveryLedgerRecord; reason: string; @@ -8682,12 +8672,6 @@ export class TeamProvisioningService { } } - private selectOpenCodeRuntimeDeliveryNotificationReason( - record: OpenCodePromptDeliveryLedgerRecord - ): string | null { - return selectOpenCodeRuntimeDeliveryReason(record); - } - async scanOpenCodePromptDeliveryWatchdog(teamName: string): Promise { if (!this.isOpenCodePromptDeliveryWatchdogEnabled()) { return 0; @@ -12256,7 +12240,9 @@ export class TeamProvisioningService { .catch(() => []); const record = records.find((candidate) => candidate.inboxMessageId === normalizedMessageId); if (record) { - return this.toOpenCodeRuntimeDeliveryStatus(record); + const { record: latestRecord, decision } = + await this.decideOpenCodeRuntimeDeliveryUserFacingAdvisory(record); + return this.toOpenCodeRuntimeDeliveryStatus(latestRecord, decision); } } return null; @@ -12395,13 +12381,75 @@ export class TeamProvisioningService { }); } + buildOpenCodeRuntimeDeliveryUserVisibleImpact(input: { + delivered?: boolean; + responsePending?: boolean; + acceptanceUnknown?: boolean; + responseState?: OpenCodeMemberInboxDelivery['responseState']; + ledgerStatus?: OpenCodePromptDeliveryStatus; + reason?: string; + diagnostics?: string[]; + queuedBehindMessageId?: string; + policyImpact?: OpenCodeRuntimeDeliveryUserVisibleImpact; + }): OpenCodeRuntimeDeliveryUserVisibleImpact { + if (input.policyImpact) { + return input.policyImpact; + } + if ( + input.responsePending === true || + input.acceptanceUnknown === true || + Boolean(input.queuedBehindMessageId) + ) { + return { + state: 'checking', + reasonCode: input.reason + ? classifyOpenCodeRuntimeDeliveryReasonCode(input.reason) + : undefined, + message: input.reason, + }; + } + if (input.delivered === false) { + const reason = input.reason ?? input.diagnostics?.find((diagnostic) => diagnostic.trim()); + if ( + input.ledgerStatus === 'failed_terminal' && + isDeferredGenericOpenCodeRuntimeDeliveryReason(reason) + ) { + return { + state: 'checking', + reasonCode: classifyOpenCodeRuntimeDeliveryReasonCode(reason), + message: reason, + }; + } + return { + state: 'error', + reasonCode: classifyOpenCodeRuntimeDeliveryReasonCode(reason), + message: reason, + }; + } + return input.policyImpact ?? { state: 'none' }; + } + private toOpenCodeRuntimeDeliveryStatus( - record: OpenCodePromptDeliveryLedgerRecord + record: OpenCodePromptDeliveryLedgerRecord, + decision?: OpenCodeRuntimeDeliveryAdvisoryDecision ): OpenCodeRuntimeDeliveryStatus { const failed = record.status === 'failed_terminal'; const responded = record.status === 'responded' && Boolean(record.inboxReadCommittedAt || record.visibleReplyMessageId); + const policyImpact = decision + ? toOpenCodeRuntimeDeliveryUserVisibleImpact(decision) + : undefined; + const userVisibleImpact = this.buildOpenCodeRuntimeDeliveryUserVisibleImpact({ + delivered: !failed, + responsePending: !failed && !responded, + acceptanceUnknown: record.acceptanceUnknown, + responseState: record.responseState, + ledgerStatus: record.status, + reason: record.lastReason ?? undefined, + diagnostics: record.diagnostics, + policyImpact, + }); return { messageId: record.inboxMessageId, providerId: 'opencode', @@ -12415,6 +12463,7 @@ export class TeamProvisioningService { acceptanceUnknown: record.acceptanceUnknown, reason: record.lastReason ?? undefined, diagnostics: record.diagnostics, + userVisibleImpact, }; } diff --git a/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryAdvisoryPolicy.ts b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryAdvisoryPolicy.ts new file mode 100644 index 00000000..f83cbb05 --- /dev/null +++ b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryAdvisoryPolicy.ts @@ -0,0 +1,364 @@ +import { + isActionRequiredOpenCodeRuntimeDeliveryReason, + normalizeOpenCodeRuntimeDeliveryDiagnostic, + selectOpenCodeRuntimeDeliveryReason, +} from './OpenCodeRuntimeDeliveryDiagnostics'; + +import type { OpenCodePromptDeliveryLedgerRecord } from './OpenCodePromptDeliveryLedger'; +import type { + MemberRuntimeAdvisory, + OpenCodeRuntimeDeliveryUserVisibleImpact, +} from '@shared/types'; + +export const OPENCODE_RUNTIME_DELIVERY_GENERIC_PROOF_GRACE_MS = 120_000; + +export interface OpenCodeRuntimeDeliveryProofSnapshot { + latestSuccessAt?: number; + visibleReplyAt?: number; + visibleReplyMessageId?: string; + visibleReplyInbox?: string; + taskProgressAt?: number; +} + +export type OpenCodeRuntimeDeliveryAdvisoryAction = 'suppress' | 'defer' | 'surface'; +export type OpenCodeRuntimeDeliveryAdvisorySeverity = 'warning' | 'error'; + +export interface OpenCodeRuntimeDeliveryAdvisoryDecision { + action: OpenCodeRuntimeDeliveryAdvisoryAction; + reason?: string; + reasonCode?: MemberRuntimeAdvisory['reasonCode']; + severity?: OpenCodeRuntimeDeliveryAdvisorySeverity; + observedAt?: string; + nextReviewAt?: string; +} + +const QUOTA_EXHAUSTED_TOKENS = [ + 'exhausted your capacity', + 'capacity exceeded', + 'quota exceeded', + 'quota exhausted', + 'insufficient credits', + 'key limit exceeded', + 'total limit', +] as const; +const RATE_LIMITED_TOKENS = [ + 'rate limit', + 'too many requests', + '429', + 'model cooldown', + 'cooling down', +] as const; +const AUTH_ERROR_TOKENS = [ + 'auth_unavailable', + 'no auth available', + 'authentication_failed', + 'unauthorized', + 'forbidden', + 'invalid api key', + 'authentication', + 'api key', + 'does not have access', + 'please run /login', +] as const; +const CODEX_NATIVE_TIMEOUT_TOKENS = ['codex native exec timed out'] as const; +const NETWORK_ERROR_TOKENS = [ + 'timeout', + 'timed out', + 'network', + 'connection', + 'econn', + 'enotfound', + 'fetch failed', +] as const; +const PROVIDER_OVERLOADED_TOKENS = [ + 'overloaded', + 'temporarily unavailable', + 'service unavailable', + '503', +] as const; +const PROTOCOL_PROOF_MISSING_TOKENS = [ + 'non_visible_tool_without_task_progress', + 'visible_reply_still_required', + 'visible_reply_ack_only_still_requires_answer', + 'plain_text_ack_only_still_requires_answer', + 'visible_reply_destination_not_found_yet', + 'visible_reply_missing_relayofmessageid', + 'visible_reply_missing_task_refs', + 'visible_reply_missing_task_refs_after_merge', + 'visible_reply_task_refs_merge_failed', + 'did not create a visible reply', + 'did not create a visible message_send reply', + 'did not create a visible reply or task progress proof', + 'without the required relayofmessageid correlation', + 'without the required taskrefs metadata', + 'could not be verified', + 'no visible reply has been found yet', +] as const; +const DEFERRED_GENERIC_DELIVERY_TOKENS = [ + ...PROTOCOL_PROOF_MISSING_TOKENS, + 'empty_assistant_turn', + 'empty assistant turn', + 'prompt_delivered_no_assistant_message', + 'accepted the prompt, but no assistant turn was recorded', + 'opencode runtime delivery did not complete', + 'opencode message delivery observe bridge failed', + 'opencode bridge command timed out', + 'opencode app mcp was reattached before message delivery', + 'reattached stale opencode app mcp server', + 'recreated opencode session before message delivery', + 'opencode session reconcile skipped because the stored session is stale', +] as const; + +const HARD_RUNTIME_RESPONSE_STATES = new Set([ + 'session_error', + 'tool_error', + 'permission_blocked', + 'reconcile_failed', +]); + +function includesAnyToken(value: string, tokens: readonly string[]): boolean { + return tokens.some((token) => value.includes(token)); +} + +function normalizeForClassification(message: string | null | undefined): string { + return normalizeOpenCodeRuntimeDeliveryDiagnostic(message)?.toLowerCase() ?? ''; +} + +export function classifyOpenCodeRuntimeDeliveryReasonCode( + message: string | undefined +): MemberRuntimeAdvisory['reasonCode'] { + const normalized = normalizeForClassification(message); + if (!normalized) { + return 'unknown'; + } + if (includesAnyToken(normalized, QUOTA_EXHAUSTED_TOKENS)) { + return 'quota_exhausted'; + } + if (includesAnyToken(normalized, RATE_LIMITED_TOKENS)) { + return 'rate_limited'; + } + if (includesAnyToken(normalized, AUTH_ERROR_TOKENS)) { + return 'auth_error'; + } + if (includesAnyToken(normalized, CODEX_NATIVE_TIMEOUT_TOKENS)) { + return 'codex_native_timeout'; + } + if (includesAnyToken(normalized, NETWORK_ERROR_TOKENS)) { + return 'network_error'; + } + if (includesAnyToken(normalized, PROVIDER_OVERLOADED_TOKENS)) { + return 'provider_overloaded'; + } + if (includesAnyToken(normalized, PROTOCOL_PROOF_MISSING_TOKENS)) { + return 'protocol_proof_missing'; + } + return 'backend_error'; +} + +export function getOpenCodeRuntimeDeliveryRecordTimeMs( + record: OpenCodePromptDeliveryLedgerRecord +): number { + const candidates = [ + record.failedAt, + record.respondedAt, + record.lastObservedAt, + record.updatedAt, + record.createdAt, + ]; + for (const candidate of candidates) { + const time = Date.parse(candidate ?? ''); + if (Number.isFinite(time)) { + return time; + } + } + return 0; +} + +export function getOpenCodeRuntimeDeliveryPromptTimeMs( + record: OpenCodePromptDeliveryLedgerRecord +): number { + const candidates = [record.inboxTimestamp, record.acceptedAt, record.createdAt, record.updatedAt]; + for (const candidate of candidates) { + const time = Date.parse(candidate ?? ''); + if (Number.isFinite(time)) { + return time; + } + } + return getOpenCodeRuntimeDeliveryRecordTimeMs(record); +} + +export function isTerminalSuccessfulOpenCodeDeliveryRecord( + record: OpenCodePromptDeliveryLedgerRecord +): boolean { + return ( + record.status === 'responded' && + Boolean(record.inboxReadCommittedAt || record.visibleReplyMessageId) + ); +} + +export function isPotentialOpenCodeRuntimeDeliveryError( + record: OpenCodePromptDeliveryLedgerRecord +): boolean { + if (record.status === 'failed_terminal') { + return true; + } + return ( + record.status !== 'responded' && + (record.responseState === 'session_error' || + record.responseState === 'tool_error' || + record.responseState === 'permission_blocked' || + record.responseState === 'reconcile_failed') + ); +} + +export function isProofOnlyOpenCodeRuntimeDeliveryReason( + reason: string | null | undefined +): boolean { + return ( + classifyOpenCodeRuntimeDeliveryReasonCode(reason ?? undefined) === 'protocol_proof_missing' + ); +} + +export function isDeferredGenericOpenCodeRuntimeDeliveryReason( + reason: string | null | undefined +): boolean { + const normalized = normalizeForClassification(reason); + return Boolean(normalized) && includesAnyToken(normalized, DEFERRED_GENERIC_DELIVERY_TOKENS); +} + +export function isHardOpenCodeRuntimeDeliveryReason(input: { + record: OpenCodePromptDeliveryLedgerRecord; + reason: string | null | undefined; +}): boolean { + if (isActionRequiredOpenCodeRuntimeDeliveryReason(input.reason)) { + return true; + } + if (input.record.status !== 'failed_terminal') { + return input.record.responseState === 'permission_blocked'; + } + if (isDeferredGenericOpenCodeRuntimeDeliveryReason(input.reason)) { + return false; + } + if (input.record.responseState && HARD_RUNTIME_RESPONSE_STATES.has(input.record.responseState)) { + return true; + } + return ( + classifyOpenCodeRuntimeDeliveryReasonCode(input.reason ?? undefined) !== + 'protocol_proof_missing' + ); +} + +export function hasSupersedingOpenCodeRuntimeDeliveryProof(input: { + record: OpenCodePromptDeliveryLedgerRecord; + proof?: OpenCodeRuntimeDeliveryProofSnapshot | null; +}): boolean { + const proof = input.proof; + if (!proof) { + return false; + } + const recordTime = getOpenCodeRuntimeDeliveryRecordTimeMs(input.record); + if (typeof proof.latestSuccessAt === 'number' && proof.latestSuccessAt > recordTime) { + return true; + } + if (typeof proof.visibleReplyAt === 'number' && proof.visibleReplyAt > 0) { + return true; + } + if (typeof proof.taskProgressAt === 'number' && proof.taskProgressAt > 0) { + return true; + } + return false; +} + +export function decideOpenCodeRuntimeDeliveryAdvisory(input: { + record: OpenCodePromptDeliveryLedgerRecord; + proof?: OpenCodeRuntimeDeliveryProofSnapshot | null; + now?: number; + graceMs?: number; +}): OpenCodeRuntimeDeliveryAdvisoryDecision { + const reason = selectOpenCodeRuntimeDeliveryReason(input.record); + if (!reason) { + return { action: 'suppress' }; + } + if (hasSupersedingOpenCodeRuntimeDeliveryProof(input)) { + return { action: 'suppress' }; + } + + const now = input.now ?? Date.now(); + const graceMs = input.graceMs ?? OPENCODE_RUNTIME_DELIVERY_GENERIC_PROOF_GRACE_MS; + const recordTime = getOpenCodeRuntimeDeliveryRecordTimeMs(input.record); + const observedAt = new Date( + Number.isFinite(recordTime) && recordTime > 0 ? recordTime : now + ).toISOString(); + const reasonCode = classifyOpenCodeRuntimeDeliveryReasonCode(reason); + + if (isHardOpenCodeRuntimeDeliveryReason({ record: input.record, reason })) { + return { + action: 'surface', + severity: 'error', + reason, + reasonCode, + observedAt, + }; + } + + if (input.record.status !== 'failed_terminal') { + return { action: 'suppress' }; + } + + if ( + reasonCode === 'protocol_proof_missing' || + isDeferredGenericOpenCodeRuntimeDeliveryReason(reason) + ) { + const terminalAt = getOpenCodeRuntimeDeliveryRecordTimeMs(input.record); + const nextReviewAtMs = + Number.isFinite(terminalAt) && terminalAt > 0 ? terminalAt + graceMs : now + graceMs; + if (now < nextReviewAtMs) { + return { + action: 'defer', + reason, + reasonCode, + observedAt, + nextReviewAt: new Date(nextReviewAtMs).toISOString(), + }; + } + return { + action: 'surface', + severity: reasonCode === 'protocol_proof_missing' ? 'warning' : 'error', + reason, + reasonCode, + observedAt, + }; + } + + return { + action: 'surface', + severity: 'error', + reason, + reasonCode, + observedAt, + }; +} + +export function toOpenCodeRuntimeDeliveryUserVisibleImpact( + decision: OpenCodeRuntimeDeliveryAdvisoryDecision +): OpenCodeRuntimeDeliveryUserVisibleImpact { + if (decision.action === 'suppress') { + return { state: 'none' }; + } + if (decision.action === 'defer') { + return { + state: 'checking', + reasonCode: decision.reasonCode, + message: decision.reason, + observedAt: decision.observedAt, + nextReviewAt: decision.nextReviewAt, + }; + } + return { + state: decision.severity === 'warning' ? 'warning' : 'error', + reasonCode: decision.reasonCode, + message: decision.reason, + observedAt: decision.observedAt, + nextReviewAt: decision.nextReviewAt, + }; +} diff --git a/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryDiagnostics.ts b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryDiagnostics.ts index f97d5cb1..835e5101 100644 --- a/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryDiagnostics.ts +++ b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryDiagnostics.ts @@ -105,12 +105,13 @@ function getOpenCodeRuntimeDeliveryStateFallback( ): string | null { const state = record.responseState?.trim(); const reason = record.lastReason?.trim(); + const normalizedReason = reason?.toLowerCase(); const diagnostics = record.diagnostics.map((diagnostic) => diagnostic.trim().toLowerCase()); - if (state === 'empty_assistant_turn' || reason === 'empty_assistant_turn') { + if (state === 'empty_assistant_turn' || normalizedReason === 'empty_assistant_turn') { return 'OpenCode returned an empty assistant turn.'; } if ( - reason === 'visible_reply_missing_task_refs' || + normalizedReason === 'visible_reply_missing_task_refs' || diagnostics.includes('visible_reply_missing_task_refs') || diagnostics.includes('visible_reply_missing_task_refs_after_merge') ) { @@ -120,25 +121,25 @@ function getOpenCodeRuntimeDeliveryStateFallback( return 'OpenCode created a reply without the required taskRefs metadata, and the app could not attach it automatically.'; } if ( - reason === 'visible_reply_still_required' || - reason === 'visible_reply_ack_only_still_requires_answer' || - reason === 'plain_text_ack_only_still_requires_answer' + normalizedReason === 'visible_reply_still_required' || + normalizedReason === 'visible_reply_ack_only_still_requires_answer' || + normalizedReason === 'plain_text_ack_only_still_requires_answer' ) { return 'OpenCode responded, but did not create a visible message_send reply.'; } if ( state === 'prompt_delivered_no_assistant_message' || - reason === 'prompt_delivered_no_assistant_message' + normalizedReason === 'prompt_delivered_no_assistant_message' ) { return 'OpenCode accepted the prompt, but no assistant turn was recorded.'; } if ( - reason === 'visible_reply_destination_not_found_yet' || - reason === 'visible_reply_missing_relayOfMessageId' + normalizedReason === 'visible_reply_destination_not_found_yet' || + normalizedReason === 'visible_reply_missing_relayofmessageid' ) { return 'OpenCode created a reply without the required relayOfMessageId correlation.'; } - if (reason === 'non_visible_tool_without_task_progress') { + if (normalizedReason === 'non_visible_tool_without_task_progress') { return 'OpenCode used tools, but did not create a visible reply or task progress proof.'; } return null; diff --git a/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryProofMatching.ts b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryProofMatching.ts new file mode 100644 index 00000000..320e2d14 --- /dev/null +++ b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryProofMatching.ts @@ -0,0 +1,163 @@ +import { + isOpenCodeVisibleReplySemanticallySufficient, + type OpenCodeVisibleReplyProof, +} from './OpenCodePromptDeliveryWatchdog'; + +import type { OpenCodePromptDeliveryLedgerRecord } from './OpenCodePromptDeliveryLedger'; +import type { InboxMessage, TaskRef } from '@shared/types'; + +export function normalizeOpenCodeRuntimeDeliveryToken(value: string | undefined): string { + return value?.trim().toLowerCase() ?? ''; +} + +export function isOpenCodeLeadReplyRecipientAlias(value: string): boolean { + const normalized = value + .trim() + .toLowerCase() + .replace(/[\s_]+/g, '-'); + return ( + normalized === 'lead' || + normalized === 'team-lead' || + normalized === 'teamlead' || + normalized === 'team-leader' + ); +} + +export function getOpenCodeVisibleReplyInboxCandidates(input: { + replyRecipient?: string | null; + configuredLeadName?: string | null; + includeUserFallbackForLeadRecipient?: boolean; +}): string[] { + const explicitRecipient = input.replyRecipient?.trim() || 'user'; + const candidates = [explicitRecipient]; + const configuredLeadName = input.configuredLeadName?.trim() || null; + const isConfiguredLeadRecipient = + Boolean(configuredLeadName) && + configuredLeadName?.toLowerCase() === explicitRecipient.toLowerCase(); + + if (isOpenCodeLeadReplyRecipientAlias(explicitRecipient) || isConfiguredLeadRecipient) { + if (configuredLeadName) { + candidates.push(configuredLeadName); + } + candidates.push('lead'); + candidates.push('team-lead'); + if (input.includeUserFallbackForLeadRecipient) { + candidates.push('user'); + } + } + + return candidates + .filter((value): value is string => Boolean(value?.trim())) + .filter( + (value, index, list) => + list.findIndex((item) => item.toLowerCase() === value.toLowerCase()) === index + ); +} + +export function isOpenCodeVisibleReplyTimestampEligible(input: { + message: Pick; + ledgerRecord: OpenCodePromptDeliveryLedgerRecord; +}): boolean { + const messageMs = Date.parse(input.message.timestamp); + const inboxMs = Date.parse(input.ledgerRecord.inboxTimestamp); + if (!Number.isFinite(messageMs) || !Number.isFinite(inboxMs)) { + return true; + } + return messageMs + 5_000 >= inboxMs; +} + +export function normalizeOpenCodeTaskRefsForComparison( + taskRefs: readonly TaskRef[] | undefined +): TaskRef[] { + if (!Array.isArray(taskRefs)) { + return []; + } + const normalized: TaskRef[] = []; + for (const rawTaskRef of taskRefs as readonly unknown[]) { + if (!rawTaskRef || typeof rawTaskRef !== 'object') { + continue; + } + const taskRef = rawTaskRef as Record; + const teamName = typeof taskRef.teamName === 'string' ? taskRef.teamName.trim() : ''; + const taskId = typeof taskRef.taskId === 'string' ? taskRef.taskId.trim() : ''; + const displayId = typeof taskRef.displayId === 'string' ? taskRef.displayId.trim() : ''; + if (teamName && taskId && displayId) { + normalized.push({ teamName, taskId, displayId }); + } + } + return normalized; +} + +export function openCodeTaskRefKey(taskRef: TaskRef): string { + return `${taskRef.teamName.trim()}\u0000${taskRef.taskId.trim()}\u0000${taskRef.displayId.trim()}`; +} + +export function openCodeTaskRefsIncludeAll( + actual: readonly TaskRef[] | undefined, + expected: readonly TaskRef[] | undefined +): boolean { + const normalizedExpected = normalizeOpenCodeTaskRefsForComparison(expected); + if (normalizedExpected.length === 0) { + return true; + } + const actualKeys = new Set( + normalizeOpenCodeTaskRefsForComparison(actual).map((taskRef) => openCodeTaskRefKey(taskRef)) + ); + return normalizedExpected.every((taskRef) => actualKeys.has(openCodeTaskRefKey(taskRef))); +} + +export function isOpenCodeRecoveredVisibleReplyCandidate(input: { + message: InboxMessage & { messageId: string }; + ledgerRecord: OpenCodePromptDeliveryLedgerRecord; + from: string; + requireTaskRefs: boolean; +}): boolean { + const expectedFrom = normalizeOpenCodeRuntimeDeliveryToken(input.from); + if (!expectedFrom || normalizeOpenCodeRuntimeDeliveryToken(input.message.from) !== expectedFrom) { + return false; + } + if (input.message.source !== undefined && input.message.source !== 'runtime_delivery') { + return false; + } + if ( + input.requireTaskRefs && + !openCodeTaskRefsIncludeAll(input.message.taskRefs, input.ledgerRecord.taskRefs) + ) { + return false; + } + if ( + !isOpenCodeVisibleReplyTimestampEligible({ + message: input.message, + ledgerRecord: input.ledgerRecord, + }) + ) { + return false; + } + return isOpenCodeVisibleReplySemanticallySufficient({ + actionMode: input.ledgerRecord.actionMode, + taskRefs: input.ledgerRecord.taskRefs, + text: input.message.text, + summary: input.message.summary, + }).sufficient; +} + +export function getOpenCodeRuntimeDeliveryMessageTimeMs( + message: Pick +): number { + const time = Date.parse(message.timestamp); + return Number.isFinite(time) ? time : 0; +} + +export function isOpenCodeVisibleReplyProofSufficient(input: { + proof: OpenCodeVisibleReplyProof; + ledgerRecord: OpenCodePromptDeliveryLedgerRecord; +}): boolean { + return ( + isOpenCodeRecoveredVisibleReplyCandidate({ + message: input.proof.message, + ledgerRecord: input.ledgerRecord, + from: input.ledgerRecord.memberName, + requireTaskRefs: false, + }) && openCodeTaskRefsIncludeAll(input.proof.message.taskRefs, input.ledgerRecord.taskRefs) + ); +} diff --git a/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryProofReader.ts b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryProofReader.ts new file mode 100644 index 00000000..6a80919e --- /dev/null +++ b/src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryProofReader.ts @@ -0,0 +1,361 @@ +import { isLeadMember } from '@shared/utils/leadDetection'; + +import { TeamConfigReader } from '../../TeamConfigReader'; +import { TeamInboxReader } from '../../TeamInboxReader'; +import { TeamTaskReader } from '../../TeamTaskReader'; + +import { + getOpenCodeRuntimeDeliveryPromptTimeMs, + getOpenCodeRuntimeDeliveryRecordTimeMs, + isTerminalSuccessfulOpenCodeDeliveryRecord, + type OpenCodeRuntimeDeliveryProofSnapshot, +} from './OpenCodeRuntimeDeliveryAdvisoryPolicy'; +import { + getOpenCodeRuntimeDeliveryMessageTimeMs, + getOpenCodeVisibleReplyInboxCandidates, + isOpenCodeRecoveredVisibleReplyCandidate, + normalizeOpenCodeRuntimeDeliveryToken, + openCodeTaskRefsIncludeAll, +} from './OpenCodeRuntimeDeliveryProofMatching'; + +import type { OpenCodePromptDeliveryLedgerRecord } from './OpenCodePromptDeliveryLedger'; +import type { InboxMessage, TeamConfig, TeamTask } from '@shared/types'; + +const PROOF_READ_CONCURRENCY = 4; + +interface IndexedVisibleReply { + inboxName: string; + message: InboxMessage & { messageId: string }; + observedAt: number; +} + +export interface OpenCodeRuntimeDeliveryProofIndex { + getSnapshot( + memberName: string, + record: OpenCodePromptDeliveryLedgerRecord + ): OpenCodeRuntimeDeliveryProofSnapshot; +} + +export interface OpenCodeRuntimeDeliveryProofReaderInput { + teamName: string; + activeMemberKeys: ReadonlySet; + recordsByMember: ReadonlyMap; +} + +interface ConfigReaderPort { + getConfigSnapshot?(teamName: string): Promise; + getConfig(teamName: string): Promise; +} + +async function mapLimit( + items: readonly T[], + limit: number, + fn: (item: T) => Promise +): Promise { + const results = new Array(items.length); + let index = 0; + const workerCount = Math.max(1, Math.min(limit, items.length)); + const workers = new Array(workerCount).fill(0).map(async () => { + while (true) { + const currentIndex = index; + index += 1; + if (currentIndex >= items.length) { + return; + } + results[currentIndex] = await fn(items[currentIndex]); + } + }); + await Promise.all(workers); + return results; +} + +function getLatestMemberTaskProgressTime(task: TeamTask, memberKey: string): number { + let latest = 0; + for (const comment of task.comments ?? []) { + if (normalizeOpenCodeRuntimeDeliveryToken(comment.author) !== memberKey) { + continue; + } + const createdAt = Date.parse(comment.createdAt); + if (Number.isFinite(createdAt)) { + latest = Math.max(latest, createdAt); + } + } + for (const event of task.historyEvents ?? []) { + if (normalizeOpenCodeRuntimeDeliveryToken(event.actor ?? '') !== memberKey) { + continue; + } + const timestamp = Date.parse(event.timestamp); + if (Number.isFinite(timestamp)) { + latest = Math.max(latest, timestamp); + } + } + return latest; +} + +function getOpenCodeTaskProgressProofKey(memberKey: string, taskId: string): string { + return `${memberKey}::task::${taskId.trim()}`; +} + +class MaterializedOpenCodeRuntimeDeliveryProofIndex implements OpenCodeRuntimeDeliveryProofIndex { + constructor( + private readonly latestSuccessTimesByMember: ReadonlyMap, + private readonly visibleRepliesByMember: ReadonlyMap, + private readonly taskProgressTimes: ReadonlyMap + ) {} + + getSnapshot( + memberName: string, + record: OpenCodePromptDeliveryLedgerRecord + ): OpenCodeRuntimeDeliveryProofSnapshot { + const memberKey = normalizeOpenCodeRuntimeDeliveryToken(memberName); + const visibleReplies = this.visibleRepliesByMember.get(memberKey) ?? []; + const relayOfMessageId = record.inboxMessageId.trim(); + const promptTime = getOpenCodeRuntimeDeliveryPromptTimeMs(record); + let visibleReply: IndexedVisibleReply | null = null; + + const expectedMessageId = record.visibleReplyMessageId?.trim(); + if (expectedMessageId) { + visibleReply = + visibleReplies.find( + (candidate) => + candidate.message.messageId === expectedMessageId && + isOpenCodeRecoveredVisibleReplyCandidate({ + message: candidate.message, + ledgerRecord: record, + from: memberName, + requireTaskRefs: false, + }) && + openCodeTaskRefsIncludeAll(candidate.message.taskRefs, record.taskRefs) + ) ?? null; + } + + if (!visibleReply && relayOfMessageId) { + visibleReply = + visibleReplies.find((candidate) => { + const messageRelayOfMessageId = + typeof candidate.message.relayOfMessageId === 'string' + ? candidate.message.relayOfMessageId.trim() + : ''; + return ( + messageRelayOfMessageId === relayOfMessageId && + isOpenCodeRecoveredVisibleReplyCandidate({ + message: candidate.message, + ledgerRecord: record, + from: memberName, + requireTaskRefs: false, + }) && + openCodeTaskRefsIncludeAll(candidate.message.taskRefs, record.taskRefs) + ); + }) ?? null; + } + + if (!visibleReply && record.taskRefs.length > 0) { + visibleReply = + visibleReplies + .filter((candidate) => + isOpenCodeRecoveredVisibleReplyCandidate({ + message: candidate.message, + ledgerRecord: record, + from: memberName, + requireTaskRefs: true, + }) + ) + .sort((left, right) => left.observedAt - right.observedAt)[0] ?? null; + } + + let taskProgressAt = 0; + for (const taskRef of record.taskRefs) { + const taskId = taskRef.taskId?.trim(); + if (!taskId) { + continue; + } + const proofAt = this.taskProgressTimes.get( + getOpenCodeTaskProgressProofKey(memberKey, taskId) + ); + if (typeof proofAt === 'number' && proofAt > promptTime) { + taskProgressAt = Math.max(taskProgressAt, proofAt); + } + } + + return { + latestSuccessAt: this.latestSuccessTimesByMember.get(memberKey), + visibleReplyAt: visibleReply?.observedAt, + visibleReplyMessageId: visibleReply?.message.messageId, + visibleReplyInbox: visibleReply?.inboxName, + taskProgressAt: taskProgressAt > 0 ? taskProgressAt : undefined, + }; + } +} + +export class OpenCodeRuntimeDeliveryProofReader { + constructor( + private readonly inboxReader = new TeamInboxReader(), + private readonly taskReader = new TeamTaskReader(), + private readonly configReader: ConfigReaderPort = new TeamConfigReader() + ) {} + + async readProofIndex( + input: OpenCodeRuntimeDeliveryProofReaderInput + ): Promise { + const [configuredLeadName, visibleRepliesByMember, taskProgressTimes] = await Promise.all([ + this.readConfiguredLeadName(input.teamName), + this.readVisibleRepliesByMember(input), + this.readTaskProgressProofTimes( + input.teamName, + input.activeMemberKeys, + input.recordsByMember + ), + ]); + + return new MaterializedOpenCodeRuntimeDeliveryProofIndex( + this.readLatestSuccessTimesByMember(input.activeMemberKeys, input.recordsByMember), + await visibleRepliesByMember(configuredLeadName), + taskProgressTimes + ); + } + + private async readConfiguredLeadName(teamName: string): Promise { + const config = + (typeof this.configReader.getConfigSnapshot === 'function' + ? await this.configReader.getConfigSnapshot(teamName) + : await this.configReader.getConfig(teamName)) ?? null; + return config?.members?.find((member) => isLeadMember(member))?.name?.trim() || null; + } + + private async readVisibleRepliesByMember( + input: OpenCodeRuntimeDeliveryProofReaderInput + ): Promise<(configuredLeadName: string | null) => Promise>> { + const candidateDescriptors = new Map< + string, + { + replyRecipient?: string | null; + includeUserFallbackForLeadRecipient?: boolean; + } + >(); + for (const records of input.recordsByMember.values()) { + for (const record of records) { + const includeUserFallbackForLeadRecipient = true; + const key = `${record.replyRecipient ?? ''}\u0000${includeUserFallbackForLeadRecipient ? '1' : '0'}`; + candidateDescriptors.set(key, { + replyRecipient: record.replyRecipient, + includeUserFallbackForLeadRecipient, + }); + } + } + + return async (configuredLeadName: string | null) => { + const inboxNames = new Set(); + for (const descriptor of candidateDescriptors.values()) { + for (const inboxName of getOpenCodeVisibleReplyInboxCandidates({ + ...descriptor, + configuredLeadName, + })) { + inboxNames.add(inboxName); + } + } + + const result = new Map(); + await mapLimit(Array.from(inboxNames), PROOF_READ_CONCURRENCY, async (inboxName) => { + const messages = await this.inboxReader + .getMessagesFor(input.teamName, inboxName) + .catch(() => []); + for (const message of messages) { + const messageId = typeof message.messageId === 'string' ? message.messageId.trim() : ''; + if (!messageId) { + continue; + } + const memberKey = normalizeOpenCodeRuntimeDeliveryToken(message.from); + if (!input.activeMemberKeys.has(memberKey)) { + continue; + } + if (message.source !== undefined && message.source !== 'runtime_delivery') { + continue; + } + const observedAt = getOpenCodeRuntimeDeliveryMessageTimeMs(message); + const existing = result.get(memberKey) ?? []; + existing.push({ + inboxName, + message: { ...message, messageId }, + observedAt, + }); + result.set(memberKey, existing); + } + }); + return result; + }; + } + + private readLatestSuccessTimesByMember( + activeMemberKeys: ReadonlySet, + recordsByMember: ReadonlyMap + ): Map { + const result = new Map(); + for (const [memberKey, records] of recordsByMember) { + if (!activeMemberKeys.has(memberKey)) { + continue; + } + for (const record of records) { + if (!isTerminalSuccessfulOpenCodeDeliveryRecord(record)) { + continue; + } + result.set( + memberKey, + Math.max(result.get(memberKey) ?? 0, getOpenCodeRuntimeDeliveryRecordTimeMs(record)) + ); + } + } + return result; + } + + private async readTaskProgressProofTimes( + teamName: string, + activeMemberKeys: ReadonlySet, + recordsByMember: ReadonlyMap + ): Promise> { + const taskIdsByMember = new Map>(); + for (const [memberKey, records] of recordsByMember) { + if (!activeMemberKeys.has(memberKey)) { + continue; + } + for (const record of records) { + for (const taskRef of record.taskRefs) { + const taskId = taskRef.taskId?.trim(); + if (!taskId) { + continue; + } + const taskIds = taskIdsByMember.get(memberKey) ?? new Set(); + taskIds.add(taskId); + taskIdsByMember.set(memberKey, taskIds); + } + } + } + if (taskIdsByMember.size === 0) { + return new Map(); + } + + const tasks = await this.taskReader.getTasks(teamName).catch(() => []); + if (tasks.length === 0) { + return new Map(); + } + + const result = new Map(); + for (const task of tasks) { + const taskId = task.id?.trim(); + if (!taskId) { + continue; + } + for (const [memberKey, taskIds] of taskIdsByMember) { + if (!taskIds.has(taskId)) { + continue; + } + const proofAt = getLatestMemberTaskProgressTime(task, memberKey); + if (proofAt <= 0) { + continue; + } + const key = getOpenCodeTaskProgressProofKey(memberKey, taskId); + result.set(key, Math.max(result.get(key) ?? 0, proofAt)); + } + } + return result; + } +} diff --git a/src/main/services/team/teamDataWorkerTypes.ts b/src/main/services/team/teamDataWorkerTypes.ts index 3a33dd64..5ef127df 100644 --- a/src/main/services/team/teamDataWorkerTypes.ts +++ b/src/main/services/team/teamDataWorkerTypes.ts @@ -48,6 +48,11 @@ export interface InvalidateTeamMessageFeedPayload { teamName: string; } +export interface InvalidateMemberRuntimeAdvisoryPayload { + teamName: string; + memberName?: string; +} + export interface TeamDataWorkerDiag { op: TeamDataWorkerRequest['op']; teamName?: string; @@ -64,7 +69,12 @@ export type TeamDataWorkerRequest = | { id: string; op: 'getMemberActivityMeta'; payload: GetMemberActivityMetaPayload } | { id: string; op: 'findLogsForTask'; payload: FindLogsForTaskPayload } | { id: string; op: 'invalidateTeamConfig'; payload: InvalidateTeamConfigPayload } - | { id: string; op: 'invalidateTeamMessageFeed'; payload: InvalidateTeamMessageFeedPayload }; + | { id: string; op: 'invalidateTeamMessageFeed'; payload: InvalidateTeamMessageFeedPayload } + | { + id: string; + op: 'invalidateMemberRuntimeAdvisory'; + payload: InvalidateMemberRuntimeAdvisoryPayload; + }; export type TeamDataWorkerResponse = | { diff --git a/src/main/workers/team-data-worker.ts b/src/main/workers/team-data-worker.ts index ea4d9f58..78c87da4 100644 --- a/src/main/workers/team-data-worker.ts +++ b/src/main/workers/team-data-worker.ts @@ -70,6 +70,7 @@ parentPort?.on('message', async (msg: TeamDataWorkerRequest) => { case 'invalidateTeamConfig': { TeamConfigReader.invalidateTeam(msg.payload.teamName); teamDataService.invalidateMessageFeed(msg.payload.teamName); + teamDataService.invalidateTeamRuntimeAdvisories(msg.payload.teamName); respond({ id: msg.id, ok: true, result: null, diag: buildDiag() }); break; } @@ -78,6 +79,18 @@ parentPort?.on('message', async (msg: TeamDataWorkerRequest) => { respond({ id: msg.id, ok: true, result: null, diag: buildDiag() }); break; } + case 'invalidateMemberRuntimeAdvisory': { + if (msg.payload.memberName) { + teamDataService.invalidateMemberRuntimeAdvisory( + msg.payload.teamName, + msg.payload.memberName + ); + } else { + teamDataService.invalidateTeamRuntimeAdvisories(msg.payload.teamName); + } + respond({ id: msg.id, ok: true, result: null, diag: buildDiag() }); + break; + } case 'findLogsForTask': { const { teamName, taskId, options } = msg.payload; const intervalsKey = options?.intervals diff --git a/src/renderer/components/team/TeamChangesSection.tsx b/src/renderer/components/team/TeamChangesSection.tsx index d022f2d6..03b30a2a 100644 --- a/src/renderer/components/team/TeamChangesSection.tsx +++ b/src/renderer/components/team/TeamChangesSection.tsx @@ -4,28 +4,21 @@ import { api } from '@renderer/api'; import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip'; import { useStore } from '@renderer/store'; import { resolveTaskChangePresenceFromResult } from '@renderer/utils/taskChangePresence'; -import { - buildTaskChangeRequestOptions, - canDisplayTaskChangesForOptions, - type TaskChangeRequestOptions, -} from '@renderer/utils/taskChangeRequest'; import { deriveTaskDisplayId } from '@shared/utils/taskIdentity'; import { AlertTriangle, FileDiff, GitCompareArrows, Loader2, RefreshCw } from 'lucide-react'; import { FileIcon } from './editor/FileIcon'; import { CollapsibleTeamSection } from './CollapsibleTeamSection'; +import { + buildTeamChangeRequestPlan, + buildTeamChangesTasksFingerprint, + getTeamChangeTaskTimeMs, + TEAM_CHANGES_MAX_RENDERED_FILE_ROWS, +} from './teamChangesRequestPlan'; -import type { - FileChangeSummary, - TaskChangeSetV2, - TeamTaskChangeSummaryRequest, - TeamTaskWithKanban, -} from '@shared/types'; +import type { FileChangeSummary, TaskChangeSetV2, TeamTaskWithKanban } from '@shared/types'; const TEAM_CHANGES_AUTO_REFRESH_MS = 30_000; -const TEAM_CHANGES_MAX_REQUESTS = 120; -const TEAM_CHANGES_UNKNOWN_SCAN_LIMIT = 32; -const TEAM_CHANGES_MAX_RENDERED_FILE_ROWS = 300; interface TeamChangesSectionProps { teamName: string; @@ -33,28 +26,10 @@ interface TeamChangesSectionProps { onViewChanges: (taskId: string, filePath?: string) => void; } -interface TeamChangeCandidate { - task: TeamTaskWithKanban; - options: TaskChangeRequestOptions; - priority: number; - isUnknownScan: boolean; -} - -interface TeamChangeRequestPlan { - requests: TeamTaskChangeSummaryRequest[]; - requestOptionsByTaskId: Map; - eligibleCount: number; - requestedCount: number; - deferredCount: number; - nextUnknownScanCursor: number; -} - interface TeamChangeSummaryState { taskId: string; changeSet: TaskChangeSetV2 | null; error?: string; - options: TaskChangeRequestOptions; - loadedAt: number; } interface TeamChangeStats { @@ -63,103 +38,10 @@ interface TeamChangeStats { deferredCount: number; } -function getTaskTimeMs(task: TeamTaskWithKanban): number { - const value = task.updatedAt ?? task.createdAt; - if (!value) return 0; - const ms = new Date(value).getTime(); - return Number.isFinite(ms) ? ms : 0; -} - -function compareCandidateRecency(a: TeamChangeCandidate, b: TeamChangeCandidate): number { - const priorityDelta = a.priority - b.priority; - if (priorityDelta !== 0) return priorityDelta; - return getTaskTimeMs(b.task) - getTaskTimeMs(a.task); -} - -function rotateCandidates(items: T[], cursor: number): T[] { - if (items.length === 0) return items; - const start = cursor % items.length; - if (start === 0) return items; - return [...items.slice(start), ...items.slice(0, start)]; -} - -function buildTeamChangeRequestPlan( - tasks: TeamTaskWithKanban[], - unknownScanCursor: number, - forceFresh: boolean -): TeamChangeRequestPlan { - const primary: TeamChangeCandidate[] = []; - const active: TeamChangeCandidate[] = []; - const unknown: TeamChangeCandidate[] = []; - const seenTaskIds = new Set(); - - for (const task of tasks) { - if (!task.id || task.status === 'deleted' || seenTaskIds.has(task.id)) { - continue; - } - seenTaskIds.add(task.id); - - const options = buildTaskChangeRequestOptions(task, { summaryOnly: true }); - const presence = task.changePresence ?? 'unknown'; - const canDisplay = canDisplayTaskChangesForOptions(options); - if (!canDisplay && presence !== 'has_changes' && presence !== 'needs_attention') { - continue; - } - - if (presence === 'has_changes') { - primary.push({ task, options, priority: 0, isUnknownScan: false }); - continue; - } - if (presence === 'needs_attention') { - primary.push({ task, options, priority: 1, isUnknownScan: false }); - continue; - } - if (options.stateBucket === 'active' && options.status === 'in_progress') { - active.push({ task, options, priority: 2, isUnknownScan: false }); - continue; - } - if (presence === 'unknown') { - unknown.push({ task, options, priority: 3, isUnknownScan: true }); - } - } - - primary.sort(compareCandidateRecency); - active.sort(compareCandidateRecency); - unknown.sort(compareCandidateRecency); - - const unknownWindow = rotateCandidates(unknown, unknownScanCursor).slice( - 0, - TEAM_CHANGES_UNKNOWN_SCAN_LIMIT - ); - const selected = [...primary, ...active, ...unknownWindow].slice(0, TEAM_CHANGES_MAX_REQUESTS); - const requestOptionsByTaskId = new Map(); - const requests = selected.map((candidate) => { - const options = { - ...candidate.options, - summaryOnly: true, - forceFresh: forceFresh ? true : candidate.options.forceFresh, - }; - requestOptionsByTaskId.set(candidate.task.id, options); - return { - taskId: candidate.task.id, - options, - }; - }); - const eligibleCount = primary.length + active.length + unknown.length; - const nextUnknownScanCursor = - unknown.length > 0 - ? (unknownScanCursor + Math.min(TEAM_CHANGES_UNKNOWN_SCAN_LIMIT, unknown.length)) % - unknown.length - : 0; - - return { - requests, - requestOptionsByTaskId, - eligibleCount, - requestedCount: requests.length, - deferredCount: Math.max(0, eligibleCount - requests.length), - nextUnknownScanCursor, - }; +interface TeamChangesLoadOptions { + forceFresh?: boolean; + showSpinner?: boolean; + preserveOnError?: boolean; } function getTaskChangeContributors( @@ -199,21 +81,6 @@ function getTaskSummaryBadge(changeSet: TaskChangeSetV2 | null): string | undefi return undefined; } -function buildTasksFingerprint(tasks: TeamTaskWithKanban[]): string { - return tasks - .map((task) => - [ - task.id, - task.status, - task.owner ?? '', - task.updatedAt ?? '', - task.changePresence ?? 'unknown', - task.workIntervals?.length ?? 0, - ].join(':') - ) - .join('|'); -} - export const TeamChangesSection = memo(function TeamChangesSection({ teamName, tasks, @@ -233,12 +100,17 @@ export const TeamChangesSection = memo(function TeamChangesSection({ const [loading, setLoading] = useState(false); const [refreshing, setRefreshing] = useState(false); const [error, setError] = useState(null); + const [queuedRefreshTick, setQueuedRefreshTick] = useState(0); const hasLoadedRef = useRef(false); const requestSeqRef = useRef(0); + const activeRequestSeqRef = useRef(null); + const queuedRefreshOptionsRef = useRef(null); + const sectionOpenRef = useRef(sectionOpen); const unknownScanCursorRef = useRef(0); const lastRequestedTasksFingerprintRef = useRef(null); - const tasksFingerprint = useMemo(() => buildTasksFingerprint(tasks), [tasks]); + const tasksFingerprint = useMemo(() => buildTeamChangesTasksFingerprint(tasks), [tasks]); const taskMap = useMemo(() => new Map(tasks.map((task) => [task.id, task])), [tasks]); + sectionOpenRef.current = sectionOpen; const visibleSummaries = useMemo(() => { return Object.values(summariesByTaskId) @@ -250,7 +122,7 @@ export const TeamChangesSection = memo(function TeamChangesSection({ (entry.summary.changeSet?.files.length ?? 0) > 0 || (entry.summary.changeSet?.warnings.length ?? 0) > 0) ) - .sort((a, b) => getTaskTimeMs(b.task) - getTaskTimeMs(a.task)); + .sort((a, b) => getTeamChangeTaskTimeMs(b.task) - getTeamChangeTaskTimeMs(a.task)); }, [summariesByTaskId, taskMap]); const totalFiles = visibleSummaries.reduce( @@ -265,13 +137,24 @@ export const TeamChangesSection = memo(function TeamChangesSection({ forceFresh = false, showSpinner = false, preserveOnError = true, - }: { - forceFresh?: boolean; - showSpinner?: boolean; - preserveOnError?: boolean; - } = {}): Promise => { + }: TeamChangesLoadOptions = {}): Promise => { + if (activeRequestSeqRef.current !== null) { + const previous = queuedRefreshOptionsRef.current; + queuedRefreshOptionsRef.current = { + forceFresh: Boolean(previous?.forceFresh || forceFresh), + showSpinner: Boolean(previous?.showSpinner || showSpinner), + preserveOnError: previous + ? Boolean(previous.preserveOnError && preserveOnError) + : preserveOnError, + }; + requestSeqRef.current += 1; + return; + } + const plan = buildTeamChangeRequestPlan(tasks, unknownScanCursorRef.current, forceFresh); unknownScanCursorRef.current = plan.nextUnknownScanCursor; + const requestSeq = requestSeqRef.current + 1; + requestSeqRef.current = requestSeq; setStats({ eligibleCount: plan.eligibleCount, requestedCount: plan.requestedCount, @@ -281,16 +164,17 @@ export const TeamChangesSection = memo(function TeamChangesSection({ if (plan.requests.length === 0) { setSummariesByTaskId({}); + setLoading(false); + setRefreshing(false); return; } - const requestSeq = requestSeqRef.current + 1; - requestSeqRef.current = requestSeq; if (showSpinner) { setLoading(true); } else { setRefreshing(true); } + activeRequestSeqRef.current = requestSeq; try { const response = await api.review.getTeamTaskChangeSummaries(teamName, plan.requests); @@ -312,7 +196,7 @@ export const TeamChangesSection = memo(function TeamChangesSection({ setSummariesByTaskId((previous) => { const next: Record = {}; for (const [taskId, summary] of Object.entries(previous)) { - if (currentTaskIds.has(taskId)) { + if (currentTaskIds.has(taskId) && plan.eligibleTaskIds.has(taskId)) { next[taskId] = summary; } } @@ -323,8 +207,6 @@ export const TeamChangesSection = memo(function TeamChangesSection({ taskId: item.taskId, changeSet: item.changeSet, error: item.error, - options, - loadedAt: Date.now(), }; } return next; @@ -338,7 +220,17 @@ export const TeamChangesSection = memo(function TeamChangesSection({ } setError(err instanceof Error ? err.message : 'Failed to load team changes'); } finally { - if (requestSeqRef.current === requestSeq) { + const hasQueuedRefresh = queuedRefreshOptionsRef.current !== null; + if (activeRequestSeqRef.current === requestSeq) { + activeRequestSeqRef.current = null; + } + if (hasQueuedRefresh && activeRequestSeqRef.current === null && sectionOpenRef.current) { + setQueuedRefreshTick((value) => value + 1); + } + const shouldStopIndicators = + requestSeqRef.current === requestSeq || + (!hasQueuedRefresh && activeRequestSeqRef.current === null); + if (shouldStopIndicators) { setLoading(false); setRefreshing(false); } @@ -350,6 +242,8 @@ export const TeamChangesSection = memo(function TeamChangesSection({ useEffect(() => { hasLoadedRef.current = false; requestSeqRef.current += 1; + activeRequestSeqRef.current = null; + queuedRefreshOptionsRef.current = null; unknownScanCursorRef.current = 0; lastRequestedTasksFingerprintRef.current = null; setSummariesByTaskId({}); @@ -357,6 +251,18 @@ export const TeamChangesSection = memo(function TeamChangesSection({ setStats({ eligibleCount: 0, requestedCount: 0, deferredCount: 0 }); }, [teamName]); + useEffect(() => { + if (!sectionOpen) { + requestSeqRef.current += 1; + activeRequestSeqRef.current = null; + queuedRefreshOptionsRef.current = null; + hasLoadedRef.current = false; + lastRequestedTasksFingerprintRef.current = null; + setLoading(false); + setRefreshing(false); + } + }, [sectionOpen]); + useEffect(() => { if (!sectionOpen || hasLoadedRef.current) { return; @@ -377,12 +283,27 @@ export const TeamChangesSection = memo(function TeamChangesSection({ void loadSummaries({ showSpinner: false, preserveOnError: true }); }, [loadSummaries, sectionOpen, tasksFingerprint]); + useEffect(() => { + if (!sectionOpen || activeRequestSeqRef.current !== null) { + return; + } + const options = queuedRefreshOptionsRef.current; + if (!options) { + return; + } + queuedRefreshOptionsRef.current = null; + void loadSummaries(options); + }, [loadSummaries, queuedRefreshTick, sectionOpen]); + useEffect(() => { if (!sectionOpen) { return; } const timer = window.setInterval(() => { + if (activeRequestSeqRef.current !== null || queuedRefreshOptionsRef.current !== null) { + return; + } void loadSummaries({ showSpinner: false, preserveOnError: true }); }, TEAM_CHANGES_AUTO_REFRESH_MS); diff --git a/src/renderer/components/team/TeamDetailView.tsx b/src/renderer/components/team/TeamDetailView.tsx index 92b5a2a3..92abedc7 100644 --- a/src/renderer/components/team/TeamDetailView.tsx +++ b/src/renderer/components/team/TeamDetailView.tsx @@ -53,6 +53,7 @@ import { type TaskChangeRequestOptions, } from '@renderer/utils/taskChangeRequest'; import { buildPendingRuntimeSummaryCopy } from '@renderer/utils/teamLaunchSummaryCopy'; +import { shouldClearPendingReplyForOpenCodeRuntimeDelivery } from '@renderer/utils/openCodeRuntimeDeliveryDiagnostics'; import { stripAgentBlocks } from '@shared/constants/agentBlocks'; import { deriveContextMetrics } from '@shared/utils/contextMetrics'; import { isLeadAgentType, isLeadMember } from '@shared/utils/leadDetection'; @@ -3024,8 +3025,7 @@ export const TeamDetailView = memo(function TeamDetailView({ taskRefs, }); if ( - result?.runtimeDelivery?.attempted === true && - result.runtimeDelivery.delivered === false + shouldClearPendingReplyForOpenCodeRuntimeDelivery(result?.runtimeDelivery) ) { setPendingRepliesByMember((prev) => { if (prev[member] !== sentAtMs) return prev; diff --git a/src/renderer/components/team/__tests__/teamChangesRequestPlan.test.ts b/src/renderer/components/team/__tests__/teamChangesRequestPlan.test.ts new file mode 100644 index 00000000..52b1f7f5 --- /dev/null +++ b/src/renderer/components/team/__tests__/teamChangesRequestPlan.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest'; + +import { + buildTeamChangeRequestPlan, + buildTeamChangesTasksFingerprint, + TEAM_CHANGES_UNKNOWN_SCAN_LIMIT, +} from '../teamChangesRequestPlan'; + +import type { TeamTaskWithKanban } from '@shared/types'; + +function task(overrides: Partial & { id: string }): TeamTaskWithKanban { + const { id, subject, status, ...rest } = overrides; + return { + id, + subject: subject ?? `Task ${id}`, + status: status ?? 'pending', + ...rest, + }; +} + +describe('buildTeamChangeRequestPlan', () => { + it('scans unknown pending tasks only when they have work evidence', () => { + const plan = buildTeamChangeRequestPlan( + [ + task({ id: 'plain-pending', status: 'pending', changePresence: 'unknown' }), + task({ + id: 'worked-pending', + status: 'pending', + changePresence: 'unknown', + workIntervals: [{ startedAt: '2026-05-09T08:00:00.000Z' }], + }), + ], + 0, + false + ); + + expect(plan.requests.map((request) => request.taskId)).toEqual(['worked-pending']); + expect([...plan.eligibleTaskIds]).toEqual(['worked-pending']); + }); + + it('keeps known changed tasks even when they are currently pending', () => { + const plan = buildTeamChangeRequestPlan( + [task({ id: 'known-changed', status: 'pending', changePresence: 'has_changes' })], + 0, + false + ); + + expect(plan.requests.map((request) => request.taskId)).toEqual(['known-changed']); + expect(plan.eligibleTaskIds.has('known-changed')).toBe(true); + }); + + it('rotates unknown scans and preserves summary-only request options', () => { + const tasks = Array.from({ length: TEAM_CHANGES_UNKNOWN_SCAN_LIMIT + 4 }, (_, index) => + task({ + id: `task-${index}`, + status: 'completed', + changePresence: 'unknown', + updatedAt: `2026-05-09T08:${String(index).padStart(2, '0')}:00.000Z`, + }) + ); + + const firstPass = buildTeamChangeRequestPlan(tasks, 0, true); + const secondPass = buildTeamChangeRequestPlan(tasks, firstPass.nextUnknownScanCursor, false); + + expect(firstPass.requests).toHaveLength(TEAM_CHANGES_UNKNOWN_SCAN_LIMIT); + expect(firstPass.requests[0].options?.summaryOnly).toBe(true); + expect(firstPass.requests[0].options?.forceFresh).toBe(true); + expect(secondPass.requests[0].taskId).toBe('task-3'); + }); + + it('changes fingerprint when review state changes without timestamp changes', () => { + const baseTask = task({ + id: 'reviewing', + status: 'completed', + changePresence: 'unknown', + updatedAt: '2026-05-09T08:00:00.000Z', + reviewState: 'none', + }); + + expect(buildTeamChangesTasksFingerprint([baseTask])).not.toBe( + buildTeamChangesTasksFingerprint([{ ...baseTask, reviewState: 'review' }]) + ); + }); + + it('keeps fingerprint stable for task reorder and irrelevant history events', () => { + const first = task({ + id: 'task-a', + status: 'pending', + historyEvents: [ + { + id: 'event-created', + type: 'task_created', + timestamp: '2026-05-09T08:00:00.000Z', + status: 'pending', + }, + { + id: 'event-owner', + type: 'owner_changed', + timestamp: '2026-05-09T08:01:00.000Z', + from: 'alice', + to: 'bob', + }, + ], + }); + const second = task({ + id: 'task-b', + status: 'completed', + historyEvents: [ + { + id: 'event-status', + type: 'status_changed', + timestamp: '2026-05-09T08:02:00.000Z', + from: 'in_progress', + to: 'completed', + }, + ], + }); + + expect(buildTeamChangesTasksFingerprint([first, second])).toBe( + buildTeamChangesTasksFingerprint([ + second, + { + ...first, + historyEvents: [ + ...(first.historyEvents ?? []), + { + id: 'event-owner-2', + type: 'owner_changed', + timestamp: '2026-05-09T08:03:00.000Z', + from: 'bob', + to: 'carol', + }, + ], + }, + ]) + ); + }); +}); diff --git a/src/renderer/components/team/dialogs/SendMessageDialog.tsx b/src/renderer/components/team/dialogs/SendMessageDialog.tsx index 57e5712a..6b4dd2c1 100644 --- a/src/renderer/components/team/dialogs/SendMessageDialog.tsx +++ b/src/renderer/components/team/dialogs/SendMessageDialog.tsx @@ -27,6 +27,7 @@ import { buildReplyBlock } from '@renderer/utils/agentMessageFormatting'; import { removeChipTokenFromText } from '@renderer/utils/chipUtils'; import { formatAgentRole } from '@renderer/utils/formatAgentRole'; import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; +import { isOpenCodeRuntimeDeliveryHardUxFailure } from '@renderer/utils/openCodeRuntimeDeliveryDiagnostics'; import { extractTaskRefsFromText, stripEncodedTaskReferenceMetadata, @@ -292,10 +293,7 @@ export const SendMessageDialog = ({ ) ) .then((result) => { - if ( - result?.runtimeDelivery?.attempted === true && - result.runtimeDelivery.delivered === false - ) { + if (isOpenCodeRuntimeDeliveryHardUxFailure(result?.runtimeDelivery)) { return; } textDraft.clearDraft(); diff --git a/src/renderer/components/team/messages/MessageComposer.tsx b/src/renderer/components/team/messages/MessageComposer.tsx index 8a1b3341..06a5e2a8 100644 --- a/src/renderer/components/team/messages/MessageComposer.tsx +++ b/src/renderer/components/team/messages/MessageComposer.tsx @@ -19,6 +19,7 @@ import { isTeamProvisioningActive } from '@renderer/store/slices/teamSlice'; import { serializeChipsWithText } from '@renderer/types/inlineChip'; import { formatAgentRole } from '@renderer/utils/formatAgentRole'; import { buildMemberColorMap } from '@renderer/utils/memberHelpers'; +import { isOpenCodeRuntimeDeliveryHardUxFailureFromDebugDetails } from '@renderer/utils/openCodeRuntimeDeliveryDiagnostics'; import { nameColorSet } from '@renderer/utils/projectColor'; import { getSuggestedSlashCommandsForProvider } from '@renderer/utils/providerSlashCommands'; import { buildSlashCommandSuggestions } from '@renderer/utils/skillCommandSuggestions'; @@ -469,7 +470,9 @@ export const MessageComposer = ({ if (!hasCompletionSignal) return; pendingSendRef.current = null; - const failed = sendError !== null || sendDebugDetails?.delivered === false; + const failed = + sendError !== null || + isOpenCodeRuntimeDeliveryHardUxFailureFromDebugDetails(sendDebugDetails); if (failed) { if (!isPendingCurrentTeam) return; const currentDraftIsEmpty = diff --git a/src/renderer/components/team/messages/MessagesPanel.tsx b/src/renderer/components/team/messages/MessagesPanel.tsx index b46f3a4a..fea15833 100644 --- a/src/renderer/components/team/messages/MessagesPanel.tsx +++ b/src/renderer/components/team/messages/MessagesPanel.tsx @@ -19,6 +19,7 @@ import { useTeamMessagesExpanded } from '@renderer/hooks/useTeamMessagesExpanded import { useTeamMessagesRead } from '@renderer/hooks/useTeamMessagesRead'; import { useStore } from '@renderer/store'; import { selectTeamMessages } from '@renderer/store/slices/teamSlice'; +import { shouldClearPendingReplyForOpenCodeRuntimeDelivery } from '@renderer/utils/openCodeRuntimeDeliveryDiagnostics'; import { filterTeamMessages } from '@renderer/utils/teamMessageFiltering'; import { toMessageKey } from '@renderer/utils/teamMessageKey'; import { shouldExcludeInboxTextFromReplyCandidates } from '@shared/utils/idleNotificationSemantics'; @@ -691,12 +692,19 @@ export const MessagesPanel = memo(function MessagesPanel({ useEffect(() => { const debugDetails = sendMessageDebugDetails; const messageId = debugDetails?.messageId; - if (!messageId || sendMessageRuntimeReplyVisible || debugDetails?.responsePending !== true) { + const shouldPoll = + debugDetails?.userVisibleState === 'checking' || + (!debugDetails?.userVisibleState && debugDetails?.responsePending === true); + if (!messageId || sendMessageRuntimeReplyVisible || !shouldPoll) { return; } + const statusMessageId = debugDetails.statusMessageId || messageId; const timers = OPENCODE_RUNTIME_DELIVERY_STATUS_REFRESH_DELAYS_MS.map((delayMs) => window.setTimeout(() => { - void refreshSendMessageRuntimeDeliveryStatus(teamName, messageId); + void refreshSendMessageRuntimeDeliveryStatus(teamName, { + messageId, + statusMessageId, + }); }, delayMs) ); return () => { @@ -705,7 +713,9 @@ export const MessagesPanel = memo(function MessagesPanel({ }, [ refreshSendMessageRuntimeDeliveryStatus, sendMessageDebugDetails?.messageId, + sendMessageDebugDetails?.statusMessageId, sendMessageDebugDetails?.responsePending, + sendMessageDebugDetails?.userVisibleState, sendMessageRuntimeReplyVisible, teamName, ]); @@ -732,10 +742,7 @@ export const MessagesPanel = memo(function MessagesPanel({ taskRefs, }) .then((result) => { - if ( - result?.runtimeDelivery?.attempted === true && - result.runtimeDelivery.delivered === false - ) { + if (shouldClearPendingReplyForOpenCodeRuntimeDelivery(result?.runtimeDelivery)) { onPendingReplyChange((prev) => { if (prev[member] !== sentAtMs) return prev; const next = { ...prev }; diff --git a/src/renderer/components/team/messages/OpenCodeDeliveryWarning.tsx b/src/renderer/components/team/messages/OpenCodeDeliveryWarning.tsx index 8d1e2197..18a304b8 100644 --- a/src/renderer/components/team/messages/OpenCodeDeliveryWarning.tsx +++ b/src/renderer/components/team/messages/OpenCodeDeliveryWarning.tsx @@ -19,9 +19,12 @@ export const OpenCodeDeliveryWarning = ({ debugDetails, pendingDelayMs = 10_000, }: OpenCodeDeliveryWarningProps): JSX.Element | null => { - const detailsKey = `${warning ?? ''}:${debugDetails?.messageId ?? ''}`; + const detailsKey = `${warning ?? ''}:${debugDetails?.messageId ?? ''}:${debugDetails?.statusMessageId ?? ''}:${debugDetails?.userVisibleState ?? ''}`; const delayPendingWarning = - debugDetails?.responsePending === true && debugDetails.delivered !== false; + debugDetails?.userVisibleState === 'checking' || + (!debugDetails?.userVisibleState && + debugDetails?.responsePending === true && + debugDetails.delivered !== false); const [expandedKey, setExpandedKey] = useState(null); const [copiedKey, setCopiedKey] = useState(null); const [pendingVisibleKey, setPendingVisibleKey] = useState(() => @@ -118,6 +121,8 @@ export const OpenCodeDeliveryWarning = ({ messageId {debugDetails.messageId} + statusMessageId + {debugDetails.statusMessageId} providerId {debugDetails.providerId} delivered @@ -128,8 +133,22 @@ export const OpenCodeDeliveryWarning = ({ {debugDetails.responseState ?? 'null'} ledgerStatus {debugDetails.ledgerStatus ?? 'null'} + visibleReplyMessageId + {debugDetails.visibleReplyMessageId ?? 'null'} + visibleReplyCorrelation + {debugDetails.visibleReplyCorrelation ?? 'null'} + queuedBehindMessageId + {debugDetails.queuedBehindMessageId ?? 'null'} acceptanceUnknown {String(debugDetails.acceptanceUnknown)} + userVisibleState + {debugDetails.userVisibleState ?? 'null'} + userVisibleReasonCode + {debugDetails.userVisibleReasonCode ?? 'null'} + userVisibleNextReviewAt + {debugDetails.userVisibleNextReviewAt ?? 'null'} + userVisibleMessage + {debugDetails.userVisibleMessage ?? 'null'} reason {debugDetails.reason ?? 'null'} diagnostics diff --git a/src/renderer/components/team/teamChangesRequestPlan.ts b/src/renderer/components/team/teamChangesRequestPlan.ts new file mode 100644 index 00000000..f057d3d4 --- /dev/null +++ b/src/renderer/components/team/teamChangesRequestPlan.ts @@ -0,0 +1,193 @@ +import { + buildTaskChangeRequestOptions, + canDisplayTaskChangesForOptions, +} from '@renderer/utils/taskChangeRequest'; + +import type { + TaskChangeRequestOptions, + TeamTaskChangeSummaryRequest, + TeamTaskWithKanban, +} from '@shared/types'; + +export const TEAM_CHANGES_MAX_REQUESTS = 120; +export const TEAM_CHANGES_UNKNOWN_SCAN_LIMIT = 32; +export const TEAM_CHANGES_MAX_RENDERED_FILE_ROWS = 300; + +interface TeamChangeCandidate { + task: TeamTaskWithKanban; + options: TaskChangeRequestOptions; + priority: number; +} + +export interface TeamChangeRequestPlan { + requests: TeamTaskChangeSummaryRequest[]; + requestOptionsByTaskId: Map; + eligibleTaskIds: Set; + eligibleCount: number; + requestedCount: number; + deferredCount: number; + nextUnknownScanCursor: number; +} + +export function getTeamChangeTaskTimeMs(task: TeamTaskWithKanban): number { + const value = task.updatedAt ?? task.createdAt; + if (!value) return 0; + const ms = new Date(value).getTime(); + return Number.isFinite(ms) ? ms : 0; +} + +function compareCandidateRecency(a: TeamChangeCandidate, b: TeamChangeCandidate): number { + const priorityDelta = a.priority - b.priority; + if (priorityDelta !== 0) return priorityDelta; + return getTeamChangeTaskTimeMs(b.task) - getTeamChangeTaskTimeMs(a.task); +} + +function rotateCandidates(items: T[], cursor: number): T[] { + if (items.length === 0) return items; + const start = cursor % items.length; + if (start === 0) return items; + return [...items.slice(start), ...items.slice(0, start)]; +} + +function hasTaskChangeScanEvidence(task: TeamTaskWithKanban): boolean { + if ((task.workIntervals?.length ?? 0) > 0 || (task.reviewIntervals?.length ?? 0) > 0) { + return true; + } + return ( + task.historyEvents?.some((event) => { + if (event.type === 'task_created') { + return false; + } + return event.type === 'status_changed' || event.type.startsWith('review_'); + }) ?? false + ); +} + +function getRelevantHistoryEvents(task: TeamTaskWithKanban): { type: string; timestamp: string }[] { + return ( + task.historyEvents + ?.filter((event) => event.type === 'status_changed' || event.type.startsWith('review_')) + .map((event) => ({ + type: event.type, + timestamp: event.timestamp, + })) ?? [] + ); +} + +export function buildTeamChangeRequestPlan( + tasks: TeamTaskWithKanban[], + unknownScanCursor: number, + forceFresh: boolean +): TeamChangeRequestPlan { + const primary: TeamChangeCandidate[] = []; + const active: TeamChangeCandidate[] = []; + const unknown: TeamChangeCandidate[] = []; + const seenTaskIds = new Set(); + + for (const task of tasks) { + if (!task.id || task.status === 'deleted' || seenTaskIds.has(task.id)) { + continue; + } + seenTaskIds.add(task.id); + + const options = buildTaskChangeRequestOptions(task, { summaryOnly: true }); + const presence = task.changePresence ?? 'unknown'; + const canDisplay = canDisplayTaskChangesForOptions(options); + const shouldScanUnknown = + presence === 'unknown' && (canDisplay || hasTaskChangeScanEvidence(task)); + if ( + !canDisplay && + presence !== 'has_changes' && + presence !== 'needs_attention' && + !shouldScanUnknown + ) { + continue; + } + + if (presence === 'has_changes') { + primary.push({ task, options, priority: 0 }); + continue; + } + if (presence === 'needs_attention') { + primary.push({ task, options, priority: 1 }); + continue; + } + if (options.stateBucket === 'active' && options.status === 'in_progress') { + active.push({ task, options, priority: 2 }); + continue; + } + if (shouldScanUnknown) { + unknown.push({ task, options, priority: 3 }); + } + } + + primary.sort(compareCandidateRecency); + active.sort(compareCandidateRecency); + unknown.sort(compareCandidateRecency); + + const eligibleTaskIds = new Set( + [...primary, ...active, ...unknown].map((candidate) => candidate.task.id) + ); + const unknownWindow = rotateCandidates(unknown, unknownScanCursor).slice( + 0, + TEAM_CHANGES_UNKNOWN_SCAN_LIMIT + ); + const selected = [...primary, ...active, ...unknownWindow].slice(0, TEAM_CHANGES_MAX_REQUESTS); + const requestOptionsByTaskId = new Map(); + const requests = selected.map((candidate) => { + const options = { + ...candidate.options, + summaryOnly: true, + forceFresh: forceFresh ? true : candidate.options.forceFresh, + }; + requestOptionsByTaskId.set(candidate.task.id, options); + return { + taskId: candidate.task.id, + options, + }; + }); + const eligibleCount = primary.length + active.length + unknown.length; + const nextUnknownScanCursor = + unknown.length > 0 + ? (unknownScanCursor + Math.min(TEAM_CHANGES_UNKNOWN_SCAN_LIMIT, unknown.length)) % + unknown.length + : 0; + + return { + requests, + requestOptionsByTaskId, + eligibleTaskIds, + eligibleCount, + requestedCount: requests.length, + deferredCount: Math.max(0, eligibleCount - requests.length), + nextUnknownScanCursor, + }; +} + +export function buildTeamChangesTasksFingerprint(tasks: TeamTaskWithKanban[]): string { + return JSON.stringify( + tasks + .map((task) => ({ + id: task.id, + status: task.status, + owner: task.owner ?? '', + updatedAt: task.updatedAt ?? '', + changePresence: task.changePresence ?? 'unknown', + reviewState: task.reviewState ?? '', + kanbanColumn: task.kanbanColumn ?? '', + workIntervals: + task.workIntervals?.map((interval) => ({ + startedAt: interval.startedAt, + completedAt: interval.completedAt ?? '', + })) ?? [], + reviewIntervals: + task.reviewIntervals?.map((interval) => ({ + reviewer: interval.reviewer, + startedAt: interval.startedAt, + completedAt: interval.completedAt ?? '', + })) ?? [], + historyEvents: getRelevantHistoryEvents(task), + })) + .sort((a, b) => a.id.localeCompare(b.id)) + ); +} diff --git a/src/renderer/store/slices/teamSlice.ts b/src/renderer/store/slices/teamSlice.ts index eae96093..146b4bcb 100644 --- a/src/renderer/store/slices/teamSlice.ts +++ b/src/renderer/store/slices/teamSlice.ts @@ -1,6 +1,9 @@ import { api } from '@renderer/api'; import { mergeTeamMessages } from '@renderer/utils/mergeTeamMessages'; -import { buildOpenCodeRuntimeDeliveryDiagnostics } from '@renderer/utils/openCodeRuntimeDeliveryDiagnostics'; +import { + buildOpenCodeRuntimeDeliveryDiagnostics, + isOpenCodeRuntimeDeliveryHardUxFailure, +} from '@renderer/utils/openCodeRuntimeDeliveryDiagnostics'; import { normalizePath } from '@renderer/utils/pathNormalize'; import { buildTaskChangePresenceKey, @@ -2425,7 +2428,10 @@ export interface TeamSlice { sendMessageDebugDetails: OpenCodeRuntimeDeliveryDebugDetails | null; lastSendMessageResult: SendMessageResult | null; clearSendMessageRuntimeDiagnostics: (messageId?: string | null) => void; - refreshSendMessageRuntimeDeliveryStatus: (teamName: string, messageId: string) => Promise; + refreshSendMessageRuntimeDeliveryStatus: ( + teamName: string, + input: string | { messageId: string; statusMessageId?: string | null } + ) => Promise; reviewActionError: string | null; provisioningRuns: Record; /** Synthetic TeamSummary snapshots for teams currently being provisioned (before config.json exists). */ @@ -4494,8 +4500,7 @@ export const createTeamSlice: StateCreator = (set, const result = await unwrapIpc('team:sendMessage', () => api.teams.sendMessage(teamName, request) ); - const runtimeDeliveryFailed = - result.runtimeDelivery?.attempted === true && result.runtimeDelivery.delivered === false; + const runtimeDeliveryFailed = isOpenCodeRuntimeDeliveryHardUxFailure(result.runtimeDelivery); const runtimeDeliveryDiagnostics = buildOpenCodeRuntimeDeliveryDiagnostics(result); const optimisticMessage: InboxMessage = { from: request.from ?? 'user', @@ -4563,14 +4568,29 @@ export const createTeamSlice: StateCreator = (set, }); }, - refreshSendMessageRuntimeDeliveryStatus: async (teamName: string, messageId: string) => { - const normalizedMessageId = messageId.trim(); + refreshSendMessageRuntimeDeliveryStatus: async (teamName, input) => { + const normalizedMessageId = typeof input === 'string' ? input.trim() : input.messageId.trim(); + const statusMessageId = + typeof input === 'string' + ? normalizedMessageId + : input.statusMessageId?.trim() || normalizedMessageId; if (!normalizedMessageId) return; if (get().sendMessageDebugDetails?.messageId !== normalizedMessageId) return; - const status = await unwrapIpc('team:getOpenCodeRuntimeDeliveryStatus', () => - api.teams.getOpenCodeRuntimeDeliveryStatus(teamName, normalizedMessageId) + let status = await unwrapIpc('team:getOpenCodeRuntimeDeliveryStatus', () => + api.teams.getOpenCodeRuntimeDeliveryStatus(teamName, statusMessageId) ); if (!status) return; + if (statusMessageId !== normalizedMessageId) { + const blockerStillChecking = + status.userVisibleImpact?.state === 'checking' || status.responsePending === true; + if (!blockerStillChecking) { + const ownStatus = await unwrapIpc('team:getOpenCodeRuntimeDeliveryStatus', () => + api.teams.getOpenCodeRuntimeDeliveryStatus(teamName, normalizedMessageId) + ); + if (!ownStatus) return; + status = ownStatus; + } + } const diagnostics = buildOpenCodeRuntimeDeliveryDiagnostics({ deliveredToInbox: true, messageId: normalizedMessageId, diff --git a/src/renderer/utils/openCodeRuntimeDeliveryDiagnostics.ts b/src/renderer/utils/openCodeRuntimeDeliveryDiagnostics.ts index 166840ad..55ada481 100644 --- a/src/renderer/utils/openCodeRuntimeDeliveryDiagnostics.ts +++ b/src/renderer/utils/openCodeRuntimeDeliveryDiagnostics.ts @@ -2,14 +2,22 @@ import type { SendMessageResult } from '@shared/types'; export interface OpenCodeRuntimeDeliveryDebugDetails { messageId: string; + statusMessageId?: string; providerId: string; delivered: boolean | null; responsePending: boolean | null; responseState: string | null; ledgerStatus: string | null; + visibleReplyMessageId?: string | null; + visibleReplyCorrelation?: string | null; + queuedBehindMessageId?: string | null; acceptanceUnknown: boolean | null; reason: string | null; diagnostics: string[]; + userVisibleState?: string | null; + userVisibleReasonCode?: string | null; + userVisibleMessage?: string | null; + userVisibleNextReviewAt?: string | null; } interface OpenCodeRuntimeDeliveryDiagnostics { @@ -18,7 +26,9 @@ interface OpenCodeRuntimeDeliveryDiagnostics { } const PENDING_WARNING = - 'OpenCode runtime delivery is still being checked. Message was saved and will be retried if needed.'; + 'OpenCode delivery is still being checked. Message was saved and will be observed before retry if needed.'; +const PROOF_WARNING = + 'OpenCode reply could not be verified. Message was saved to inbox, but no visible reply or task progress proof was found.'; const FAILED_WARNING = 'OpenCode runtime delivery failed. Message was saved to inbox, but live delivery did not complete.'; @@ -27,35 +37,36 @@ function formatOpenCodeRuntimeDeliveryFailureReason(reason: string | null | unde if (!normalized) { return ''; } - if (normalized === 'empty_assistant_turn') { + const normalizedLower = normalized.toLowerCase(); + if (normalizedLower === 'empty_assistant_turn') { return 'OpenCode returned an empty assistant turn.'; } - if (normalized === 'prompt_delivered_no_assistant_message') { + if (normalizedLower === 'prompt_delivered_no_assistant_message') { return 'OpenCode accepted the prompt, but no assistant turn was recorded.'; } if ( - normalized === 'visible_reply_still_required' || - normalized === 'visible_reply_ack_only_still_requires_answer' || - normalized === 'plain_text_ack_only_still_requires_answer' + normalizedLower === 'visible_reply_still_required' || + normalizedLower === 'visible_reply_ack_only_still_requires_answer' || + normalizedLower === 'plain_text_ack_only_still_requires_answer' ) { return 'OpenCode responded, but did not create a visible message_send reply.'; } if ( - normalized === 'visible_reply_destination_not_found_yet' || - normalized === 'visible_reply_missing_relayOfMessageId' + normalizedLower === 'visible_reply_destination_not_found_yet' || + normalizedLower === 'visible_reply_missing_relayofmessageid' ) { return 'OpenCode created a reply without the required relayOfMessageId correlation.'; } - if (normalized === 'visible_reply_missing_task_refs') { + if (normalizedLower === 'visible_reply_missing_task_refs') { return 'OpenCode created a reply without the required taskRefs metadata.'; } - if (normalized === 'visible_reply_missing_task_refs_after_merge') { + if (normalizedLower === 'visible_reply_missing_task_refs_after_merge') { return 'OpenCode created a reply without the required taskRefs metadata.'; } - if (normalized === 'visible_reply_task_refs_merge_failed') { + if (normalizedLower === 'visible_reply_task_refs_merge_failed') { return 'OpenCode created a reply without the required taskRefs metadata, and the app could not attach it automatically.'; } - if (normalized === 'non_visible_tool_without_task_progress') { + if (normalizedLower === 'non_visible_tool_without_task_progress') { return 'OpenCode used tools, but did not create a visible reply or task progress proof.'; } return ''; @@ -69,27 +80,42 @@ export function buildOpenCodeRuntimeDeliveryDiagnostics( return { warning: null, debugDetails: null }; } - const isFailed = runtimeDelivery.delivered === false; - const isPending = runtimeDelivery.responsePending === true; + const userVisibleState = runtimeDelivery.userVisibleImpact?.state; + const isFailed = + userVisibleState === 'error' || (!userVisibleState && runtimeDelivery.delivered === false); + const isWarning = userVisibleState === 'warning'; + const isPending = + userVisibleState === 'checking' || + (!userVisibleState && runtimeDelivery.responsePending === true); if (!isFailed && !isPending) { - return { warning: null, debugDetails: null }; + if (!isWarning) { + return { warning: null, debugDetails: null }; + } } - const failureReason = isFailed - ? formatOpenCodeRuntimeDeliveryFailureReason( - runtimeDelivery.reason ?? runtimeDelivery.diagnostics?.[0] - ) - : ''; + const userVisibleMessage = runtimeDelivery.userVisibleImpact?.message?.trim(); + const failureReason = + isFailed || isWarning + ? formatOpenCodeRuntimeDeliveryFailureReason( + userVisibleMessage ?? runtimeDelivery.reason ?? runtimeDelivery.diagnostics?.[0] + ) + : ''; + const statusMessageId = runtimeDelivery.queuedBehindMessageId ?? result.messageId; return { warning: - isFailed && failureReason - ? `${FAILED_WARNING} Reason: ${failureReason}` - : isFailed - ? FAILED_WARNING - : PENDING_WARNING, + isWarning && failureReason + ? `${PROOF_WARNING} Reason: ${failureReason}` + : isWarning + ? PROOF_WARNING + : isFailed && failureReason + ? `${FAILED_WARNING} Reason: ${failureReason}` + : isFailed + ? FAILED_WARNING + : PENDING_WARNING, debugDetails: { messageId: result.messageId, + statusMessageId, providerId: runtimeDelivery.providerId, delivered: typeof runtimeDelivery.delivered === 'boolean' ? runtimeDelivery.delivered : null, responsePending: @@ -98,30 +124,86 @@ export function buildOpenCodeRuntimeDeliveryDiagnostics( : null, responseState: runtimeDelivery.responseState ?? null, ledgerStatus: runtimeDelivery.ledgerStatus ?? null, + visibleReplyMessageId: runtimeDelivery.visibleReplyMessageId ?? null, + visibleReplyCorrelation: runtimeDelivery.visibleReplyCorrelation ?? null, + queuedBehindMessageId: runtimeDelivery.queuedBehindMessageId ?? null, acceptanceUnknown: typeof runtimeDelivery.acceptanceUnknown === 'boolean' ? runtimeDelivery.acceptanceUnknown : null, reason: runtimeDelivery.reason ?? null, diagnostics: runtimeDelivery.diagnostics ?? [], + userVisibleState: runtimeDelivery.userVisibleImpact?.state ?? null, + userVisibleReasonCode: runtimeDelivery.userVisibleImpact?.reasonCode ?? null, + userVisibleMessage: runtimeDelivery.userVisibleImpact?.message ?? null, + userVisibleNextReviewAt: runtimeDelivery.userVisibleImpact?.nextReviewAt ?? null, }, }; } +export function isOpenCodeRuntimeDeliveryHardUxFailure( + runtimeDelivery: SendMessageResult['runtimeDelivery'] | null | undefined +): boolean { + if (runtimeDelivery?.attempted !== true) { + return false; + } + const userVisibleState = runtimeDelivery.userVisibleImpact?.state; + if (userVisibleState) { + return userVisibleState === 'error'; + } + return runtimeDelivery.delivered === false; +} + +export function isOpenCodeRuntimeDeliveryHardUxFailureFromDebugDetails( + details: OpenCodeRuntimeDeliveryDebugDetails | null | undefined +): boolean { + if (!details) { + return false; + } + if (details.userVisibleState) { + return details.userVisibleState === 'error'; + } + return details.delivered === false; +} + +export function shouldClearPendingReplyForOpenCodeRuntimeDelivery( + runtimeDelivery: SendMessageResult['runtimeDelivery'] | null | undefined +): boolean { + if (runtimeDelivery?.attempted !== true) { + return false; + } + const userVisibleState = runtimeDelivery.userVisibleImpact?.state; + if (userVisibleState === 'warning' || userVisibleState === 'error') { + return true; + } + if (userVisibleState === 'checking') { + return false; + } + return runtimeDelivery.responsePending !== true; +} + export function formatOpenCodeRuntimeDeliveryDebugDetails( details: OpenCodeRuntimeDeliveryDebugDetails ): string { return JSON.stringify( { messageId: details.messageId, + statusMessageId: details.statusMessageId, providerId: details.providerId, delivered: details.delivered, responsePending: details.responsePending, responseState: details.responseState, ledgerStatus: details.ledgerStatus, + visibleReplyMessageId: details.visibleReplyMessageId, + visibleReplyCorrelation: details.visibleReplyCorrelation, + queuedBehindMessageId: details.queuedBehindMessageId, acceptanceUnknown: details.acceptanceUnknown, reason: details.reason, diagnostics: details.diagnostics, + userVisibleState: details.userVisibleState, + userVisibleReasonCode: details.userVisibleReasonCode, + userVisibleMessage: details.userVisibleMessage, + userVisibleNextReviewAt: details.userVisibleNextReviewAt, }, null, 2 diff --git a/src/shared/types/team.ts b/src/shared/types/team.ts index 6a00c75f..3b29e5c5 100644 --- a/src/shared/types/team.ts +++ b/src/shared/types/team.ts @@ -755,6 +755,7 @@ export interface SendMessageResult { queuedBehindMessageId?: string; reason?: string; diagnostics?: string[]; + userVisibleImpact?: OpenCodeRuntimeDeliveryUserVisibleImpact; }; } @@ -855,6 +856,16 @@ export interface MemberRuntimeAdvisory { statusCode?: number; } +export type OpenCodeRuntimeDeliveryUserVisibleState = 'none' | 'checking' | 'warning' | 'error'; + +export interface OpenCodeRuntimeDeliveryUserVisibleImpact { + state: OpenCodeRuntimeDeliveryUserVisibleState; + reasonCode?: MemberRuntimeAdvisory['reasonCode']; + message?: string; + observedAt?: string; + nextReviewAt?: string; +} + export interface TeamProcess { id: string; port?: number; diff --git a/test/main/services/team/ChangeExtractorService.test.ts b/test/main/services/team/ChangeExtractorService.test.ts index 019ea427..3df99970 100644 --- a/test/main/services/team/ChangeExtractorService.test.ts +++ b/test/main/services/team/ChangeExtractorService.test.ts @@ -384,6 +384,40 @@ describe('ChangeExtractorService', () => { }); }); + it('deduplicates team task change summary requests before loading', async () => { + const { service } = createService({ logPaths: [] }); + const getTaskChanges = vi + .spyOn(service, 'getTaskChanges') + .mockImplementation(async (_teamName, taskId) => makeTaskChangeResult(taskId, { taskId })); + + const response = await service.getTeamTaskChangeSummaries(TEAM_NAME, [ + { taskId: 'task-1', options: SUMMARY_OPTIONS }, + { taskId: 'task-1', options: SUMMARY_OPTIONS }, + { taskId: ' task-2 ', options: SUMMARY_OPTIONS }, + ]); + + expect(response.items.map((item) => item.taskId)).toEqual(['task-1', 'task-2']); + expect(response.truncated).toBeUndefined(); + expect(getTaskChanges).toHaveBeenCalledTimes(2); + }); + + it('ignores malformed team task change summary requests', async () => { + const { service } = createService({ logPaths: [] }); + const getTaskChanges = vi + .spyOn(service, 'getTaskChanges') + .mockImplementation(async (_teamName, taskId) => makeTaskChangeResult(taskId, { taskId })); + + const response = await service.getTeamTaskChangeSummaries(TEAM_NAME, [ + null, + { taskId: '' }, + { taskId: 'task-1', options: SUMMARY_OPTIONS }, + { taskId: 42 }, + ] as unknown as Parameters[1]); + + expect(response.items.map((item) => item.taskId)).toEqual(['task-1']); + expect(getTaskChanges).toHaveBeenCalledTimes(1); + }); + it('does not reuse detailed task-change cache across different scope inputs', async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'change-extractor-service-')); setClaudeBasePathOverride(tmpDir); diff --git a/test/main/services/team/OpenCodeRuntimeDeliveryAdvisoryPolicy.test.ts b/test/main/services/team/OpenCodeRuntimeDeliveryAdvisoryPolicy.test.ts new file mode 100644 index 00000000..fb887c9b --- /dev/null +++ b/test/main/services/team/OpenCodeRuntimeDeliveryAdvisoryPolicy.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it } from 'vitest'; + +import { + decideOpenCodeRuntimeDeliveryAdvisory, + OPENCODE_RUNTIME_DELIVERY_GENERIC_PROOF_GRACE_MS, +} from '../../../../src/main/services/team/opencode/delivery/OpenCodeRuntimeDeliveryAdvisoryPolicy'; + +import type { OpenCodePromptDeliveryLedgerRecord } from '../../../../src/main/services/team/opencode/delivery/OpenCodePromptDeliveryLedger'; + +function makeRecord( + overrides: Partial +): OpenCodePromptDeliveryLedgerRecord { + const now = '2026-05-09T12:00:00.000Z'; + return { + id: 'opencode-prompt:test', + teamName: 'team', + memberName: 'jack', + laneId: 'secondary:opencode:jack', + runId: 'run-1', + runtimeSessionId: 'session-1', + inboxMessageId: 'msg-1', + inboxTimestamp: now, + source: 'ui-send', + messageKind: null, + replyRecipient: 'user', + actionMode: null, + taskRefs: [], + payloadHash: 'sha256:test', + status: 'failed_terminal', + responseState: 'empty_assistant_turn', + attempts: 3, + maxAttempts: 3, + acceptanceUnknown: false, + nextAttemptAt: null, + lastAttemptAt: now, + lastObservedAt: now, + acceptedAt: now, + respondedAt: now, + failedAt: now, + inboxReadCommittedAt: null, + inboxReadCommitError: null, + prePromptCursor: null, + postPromptCursor: null, + deliveredUserMessageId: 'delivered-1', + observedAssistantMessageId: null, + observedAssistantPreview: null, + observedToolCallNames: [], + observedVisibleMessageId: null, + visibleReplyMessageId: null, + visibleReplyInbox: null, + visibleReplyCorrelation: null, + lastReason: 'empty_assistant_turn', + diagnostics: ['empty_assistant_turn'], + createdAt: now, + updatedAt: now, + ...overrides, + }; +} + +describe('OpenCodeRuntimeDeliveryAdvisoryPolicy', () => { + it('defers fresh generic terminal failures for proof observation', () => { + const record = makeRecord({}); + + const decision = decideOpenCodeRuntimeDeliveryAdvisory({ + record, + now: Date.parse(record.failedAt!) + 1_000, + }); + + expect(decision).toMatchObject({ + action: 'defer', + reasonCode: 'backend_error', + nextReviewAt: new Date( + Date.parse(record.failedAt!) + OPENCODE_RUNTIME_DELIVERY_GENERIC_PROOF_GRACE_MS + ).toISOString(), + }); + }); + + it('surfaces action-required failures immediately', () => { + const record = makeRecord({ + responseState: 'permission_blocked', + lastReason: 'authentication_failed', + diagnostics: ['authentication_failed'], + }); + + expect( + decideOpenCodeRuntimeDeliveryAdvisory({ + record, + now: Date.parse(record.failedAt!) + 1_000, + }) + ).toMatchObject({ + action: 'surface', + severity: 'error', + reasonCode: 'auth_error', + }); + }); + + it('suppresses generic retryable tool errors before terminal failure', () => { + const record = makeRecord({ + status: 'failed_retryable', + responseState: 'tool_error', + failedAt: null, + nextAttemptAt: '2026-05-09T12:00:30.000Z', + lastReason: 'opencode bridge command timed out', + diagnostics: ['opencode bridge command timed out'], + }); + + expect( + decideOpenCodeRuntimeDeliveryAdvisory({ + record, + now: Date.parse(record.updatedAt) + 1_000, + }) + ).toMatchObject({ action: 'suppress' }); + }); + + it('surfaces permission-blocked retryable failures before terminal failure', () => { + const record = makeRecord({ + status: 'failed_retryable', + responseState: 'permission_blocked', + failedAt: null, + nextAttemptAt: '2026-05-09T12:00:30.000Z', + lastReason: 'authentication_failed', + diagnostics: ['authentication_failed'], + }); + + expect( + decideOpenCodeRuntimeDeliveryAdvisory({ + record, + now: Date.parse(record.updatedAt) + 1_000, + }) + ).toMatchObject({ + action: 'surface', + severity: 'error', + reasonCode: 'auth_error', + }); + }); + + it('suppresses terminal failures when visible proof already exists', () => { + const record = makeRecord({}); + + expect( + decideOpenCodeRuntimeDeliveryAdvisory({ + record, + proof: { + visibleReplyAt: Date.parse(record.failedAt!) + 1_000, + visibleReplyMessageId: 'reply-1', + visibleReplyInbox: 'user', + }, + now: Date.parse(record.failedAt!) + OPENCODE_RUNTIME_DELIVERY_GENERIC_PROOF_GRACE_MS + 1, + }) + ).toMatchObject({ action: 'suppress' }); + }); +}); diff --git a/test/main/services/team/TeamMemberRuntimeAdvisoryService.test.ts b/test/main/services/team/TeamMemberRuntimeAdvisoryService.test.ts index 63382f01..887b500d 100644 --- a/test/main/services/team/TeamMemberRuntimeAdvisoryService.test.ts +++ b/test/main/services/team/TeamMemberRuntimeAdvisoryService.test.ts @@ -170,10 +170,7 @@ describe('TeamMemberRuntimeAdvisoryService', () => { ['codex_native_timeout', 'Codex native exec timed out after 120000ms.'], ['network_error', 'Fetch failed because the network connection timed out.'], ['provider_overloaded', 'Service unavailable: provider temporarily unavailable (503).'], - [ - 'protocol_proof_missing', - 'OpenCode created a reply without the required taskRefs metadata.', - ], + ['protocol_proof_missing', 'OpenCode created a reply without the required taskRefs metadata.'], ['backend_error', 'Unexpected backend blew up during request processing.'], ] as const)('classifies %s retry causes from api_error messages', async (expected, message) => { const service = new TeamMemberRuntimeAdvisoryService({} as never); @@ -372,6 +369,7 @@ describe('TeamMemberRuntimeAdvisoryService', () => { const teamName = 'relay-works'; const laneId = 'secondary:opencode:jack'; const nowIso = new Date().toISOString(); + const oldIso = new Date(Date.now() - 3 * 60 * 1000).toISOString(); const laneDir = path.join( tmpDir, 'teams', @@ -396,7 +394,7 @@ describe('TeamMemberRuntimeAdvisoryService', () => { path.join(laneDir, 'opencode-prompt-delivery-ledger.json'), JSON.stringify({ schemaVersion: 1, - updatedAt: nowIso, + updatedAt: oldIso, data: [ { id: 'opencode-prompt:proof-missing', @@ -406,7 +404,7 @@ describe('TeamMemberRuntimeAdvisoryService', () => { runId: 'run-1', runtimeSessionId: 'ses-1', inboxMessageId: 'msg-1', - inboxTimestamp: nowIso, + inboxTimestamp: oldIso, source: 'watcher', messageKind: null, replyRecipient: 'team-lead', @@ -419,11 +417,11 @@ describe('TeamMemberRuntimeAdvisoryService', () => { maxAttempts: 3, acceptanceUnknown: false, nextAttemptAt: null, - lastAttemptAt: nowIso, - lastObservedAt: nowIso, - acceptedAt: nowIso, - respondedAt: nowIso, - failedAt: nowIso, + lastAttemptAt: oldIso, + lastObservedAt: oldIso, + acceptedAt: oldIso, + respondedAt: oldIso, + failedAt: oldIso, inboxReadCommittedAt: null, inboxReadCommitError: null, prePromptCursor: null, @@ -438,8 +436,8 @@ describe('TeamMemberRuntimeAdvisoryService', () => { visibleReplyCorrelation: null, lastReason: 'non_visible_tool_without_task_progress', diagnostics: ['non_visible_tool_without_task_progress'], - createdAt: nowIso, - updatedAt: nowIso, + createdAt: oldIso, + updatedAt: oldIso, }, ], }), @@ -866,10 +864,7 @@ describe('TeamMemberRuntimeAdvisoryService', () => { ]); expect(logsFinder.findRecentMemberLogFileRefsByMember).toHaveBeenCalledTimes(1); - expect(logsFinder.findMemberLogs.mock.calls.map((call) => call[1])).toEqual([ - 'Alice', - 'Bob', - ]); + expect(logsFinder.findMemberLogs.mock.calls.map((call) => call[1])).toEqual(['Alice', 'Bob']); expect(Array.from(advisories.keys())).toEqual(['Alice', 'Bob']); }); diff --git a/test/renderer/components/team/messages/MessagesPanel.test.ts b/test/renderer/components/team/messages/MessagesPanel.test.ts index 121553bd..25e00726 100644 --- a/test/renderer/components/team/messages/MessagesPanel.test.ts +++ b/test/renderer/components/team/messages/MessagesPanel.test.ts @@ -671,10 +671,10 @@ describe('MessagesPanel idle summary invariants', () => { await Promise.resolve(); }); - expect(storeState.refreshSendMessageRuntimeDeliveryStatus).toHaveBeenCalledWith( - 'atlas-hq', - 'user-send' - ); + expect(storeState.refreshSendMessageRuntimeDeliveryStatus).toHaveBeenCalledWith('atlas-hq', { + messageId: 'user-send', + statusMessageId: 'user-send', + }); await act(async () => { root.unmount(); diff --git a/test/renderer/features/runtime-provider-management/RuntimeProviderManagementPanelView.test.ts b/test/renderer/features/runtime-provider-management/RuntimeProviderManagementPanelView.test.ts index 121308b4..10b231dc 100644 --- a/test/renderer/features/runtime-provider-management/RuntimeProviderManagementPanelView.test.ts +++ b/test/renderer/features/runtime-provider-management/RuntimeProviderManagementPanelView.test.ts @@ -721,6 +721,62 @@ describe('RuntimeProviderManagementPanelView', () => { expect(actions.useModelForNewTeams).not.toHaveBeenCalled(); }); + it('keeps the model search input enabled while model results are loading', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const actions = createActions(); + const connectedProvider = { + ...createState().view!.providers[0], + state: 'connected' as const, + ownership: ['managed'] as const, + modelCount: 174, + actions: [ + { + id: 'use' as const, + label: 'Use', + enabled: true, + disabledReason: null, + requiresSecret: false, + ownershipScope: 'runtime' as const, + }, + ], + }; + + await act(async () => { + root.render( + React.createElement(RuntimeProviderManagementPanelView, { + state: createState({ + view: { + ...createState().view!, + providers: [connectedProvider], + }, + providers: [connectedProvider], + selectedProviderId: 'openrouter', + modelPickerProviderId: 'openrouter', + modelPickerMode: 'use', + modelQuery: 'claude', + modelsLoading: true, + }), + actions, + disabled: false, + }) + ); + await Promise.resolve(); + }); + + const searchInput = host.querySelector( + '[data-testid="runtime-provider-model-search"]' + ) as HTMLInputElement | null; + + expect(searchInput).not.toBeNull(); + expect(searchInput?.disabled).toBe(false); + expect(searchInput?.value).toBe('claude'); + expect(host.querySelector('[data-testid="runtime-provider-model-loading-skeleton"]')).not.toBe( + null + ); + }); + it('keeps directory provider models visible when a model row is selected', async () => { const host = document.createElement('div'); document.body.appendChild(host); diff --git a/test/renderer/store/teamSlice.test.ts b/test/renderer/store/teamSlice.test.ts index ff96d59a..2655e2e0 100644 --- a/test/renderer/store/teamSlice.test.ts +++ b/test/renderer/store/teamSlice.test.ts @@ -477,7 +477,7 @@ describe('teamSlice actions', () => { expect(store.getState().lastSendMessageResult).toBe(result); expect(store.getState().sendMessageWarning).toBe( - 'OpenCode runtime delivery is still being checked. Message was saved and will be retried if needed.' + 'OpenCode delivery is still being checked. Message was saved and will be observed before retry if needed.' ); expect(store.getState().sendMessageDebugDetails).toMatchObject({ messageId: 'm-opencode-pending', @@ -567,7 +567,7 @@ describe('teamSlice actions', () => { store.getState().clearSendMessageRuntimeDiagnostics('other-message'); expect(store.getState().sendMessageWarning).toBe( - 'OpenCode runtime delivery is still being checked. Message was saved and will be retried if needed.' + 'OpenCode delivery is still being checked. Message was saved and will be observed before retry if needed.' ); expect(store.getState().sendMessageDebugDetails?.messageId).toBe('m-opencode-pending'); diff --git a/test/renderer/utils/openCodeRuntimeDeliveryDiagnostics.test.ts b/test/renderer/utils/openCodeRuntimeDeliveryDiagnostics.test.ts index 58123abc..cd51e658 100644 --- a/test/renderer/utils/openCodeRuntimeDeliveryDiagnostics.test.ts +++ b/test/renderer/utils/openCodeRuntimeDeliveryDiagnostics.test.ts @@ -3,6 +3,61 @@ import { describe, expect, it } from 'vitest'; import { buildOpenCodeRuntimeDeliveryDiagnostics } from '../../../src/renderer/utils/openCodeRuntimeDeliveryDiagnostics'; describe('openCodeRuntimeDeliveryDiagnostics', () => { + it('honors user-visible checking impact over raw terminal delivery facts', () => { + const diagnostics = buildOpenCodeRuntimeDeliveryDiagnostics({ + deliveredToInbox: true, + messageId: 'msg-empty', + runtimeDelivery: { + providerId: 'opencode', + attempted: true, + delivered: false, + responsePending: false, + responseState: 'empty_assistant_turn', + ledgerStatus: 'failed_terminal', + reason: 'empty_assistant_turn', + diagnostics: ['empty_assistant_turn'], + userVisibleImpact: { + state: 'checking', + reasonCode: 'backend_error', + message: 'empty_assistant_turn', + nextReviewAt: '2026-05-09T12:00:00.000Z', + }, + }, + }); + + expect(diagnostics.warning).toBe( + 'OpenCode delivery is still being checked. Message was saved and will be observed before retry if needed.' + ); + expect(diagnostics.debugDetails).toMatchObject({ + messageId: 'msg-empty', + statusMessageId: 'msg-empty', + userVisibleState: 'checking', + userVisibleNextReviewAt: '2026-05-09T12:00:00.000Z', + }); + }); + + it('honors user-visible none impact over raw terminal delivery facts', () => { + const diagnostics = buildOpenCodeRuntimeDeliveryDiagnostics({ + deliveredToInbox: true, + messageId: 'msg-proven', + runtimeDelivery: { + providerId: 'opencode', + attempted: true, + delivered: false, + responsePending: false, + responseState: 'empty_assistant_turn', + ledgerStatus: 'failed_terminal', + reason: 'empty_assistant_turn', + diagnostics: ['empty_assistant_turn'], + userVisibleImpact: { + state: 'none', + }, + }, + }); + + expect(diagnostics).toEqual({ warning: null, debugDetails: null }); + }); + it('surfaces terminal empty assistant turn in the compact failed warning', () => { const diagnostics = buildOpenCodeRuntimeDeliveryDiagnostics({ deliveredToInbox: true,