Skip to content

fix(sidebar): settle acknowledged completions to idle - #20927

Open
peeyush-ciridae wants to merge 5 commits into
stablyai:mainfrom
peeyush-ciridae:codex/opencode-v2-sidebar-idle
Open

peeyush-ciridae wants to merge 5 commits into
stablyai:mainfrom
peeyush-ciridae:codex/opencode-v2-sidebar-idle

Conversation

@peeyush-ciridae

@peeyush-ciridae peeyush-ciridae commented Sep 16, 2026 •

Copy link
Copy Markdown

ELI5

A completed agent should say Done until you have seen it. After you visit it, both the agent row and the worktree card settle back to their quiet state instead of continuing to report Done forever.

What Changed

  • Derive a presentation-only state for each sidebar agent row from its raw state and acknowledgement timestamp.
  • Keep unvisited completions as Done, show acknowledged completions as Idle, and preserve Interrupted.
  • Use the same presentation state in full rows, compact rows, and compact summaries.
  • Make worktree-level live and retained completion flags acknowledgement-aware so the card status cannot contradict its agent rows.
  • Keep the underlying terminal and agent state unchanged as done.
  • Add rendered integration coverage for full rows, compact rows, summaries, and worktree-level live and retained completion status.

Why

OpenCode can correctly finish a turn and remain in the terminal done state. Orca should preserve that completion signal until the user sees it, then present the reusable session as idle. Previously, the row remained Done indefinitely; after the initial patch, the row could become Idle while the worktree-level status still said Done. This change makes both surfaces follow the same acknowledgement boundary.

Linked Issue

Fixes #20940.

Related to #10130, which separately documents the expected reusable Codex behavior of remaining visible as Idle after completion. This PR does not claim to fix that issue's SSH row-retention path.

Visual Proof

N/A — this is a transient status transition that requires a live agent completion and acknowledgement. The accessible rendered labels and card status transition are covered by deterministic component tests instead of a static screenshot.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Verified on macOS development tooling:

  • 6 focused test files, 62 tests passed
  • renderer TypeScript project checks passed
  • changed-code quality gate passed with zero new findings
  • formatting check passed for all changed files

Manual UI testing was not performed; the transition is exercised through rendered component integration tests for full, compact, summary, live, and retained paths.

AI Disclosure

OpenAI Codex was used to inspect the status lifecycle, implement the change, add tests, and review the diff.

Review

Please focus on the acknowledgement comparison (acknowledgedAt < stateStartedAt) and the worktree summary cache invalidation when acknowledgement state changes.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: no new external input, IPC, filesystem, or network surface.
  • Cross-platform/SSH: renderer-only state projection; no platform-specific paths or commands.
  • Backwards compatibility: raw agent status remains unchanged and existing callers can omit presentation overrides.
  • Performance: acknowledgement changes rebuild the cached summary index, while structurally equal worktree summaries retain their references to avoid unrelated card renders.
  • Companion to the OpenCode v2 loader/event work in fix(opencode): support the OpenCode 2 plugin loader and event stream (#20407) #20410.

Checklist

  • This PR is small and focused
  • I explained what changed and why (including ELI5)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (focused tests, direct typecheck, formatting, and changed-code quality passed; full build was not run)

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: b4575e75-559d-46e0-910f-c9551eed01a7

📥 Commits

Reviewing files that changed from the base of the PR and between e1ee457 and 6696a41.

📒 Files selected for processing (1)
  • src/renderer/src/components/sidebar/worktree-card-agent-summary.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/renderer/src/components/sidebar/worktree-card-agent-summary.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The sidebar now computes a pane-specific display state for each agent. Unvisited completions remain done, while acknowledged completions display as idle. Interrupted agents retain interrupted. Dashboard rows, compact rows, summaries, and worktree activity status use the acknowledgement-aware state without changing the raw terminal state.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 6696a

The sidebar’s acknowledged-completion presentation remains consistent across rows, summaries, and worktree status, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: acknowledged sidebar completions settle to idle.
Description check ✅ Passed The description covers the required sections, linked issue, scope, rationale, visual-proof exception, testing, AI disclosure, review focus, platform impact, and checklist status. It clearly states tha…
Linked Issues check ✅ Passed Issue #20940 requires distinct presentation for unvisited and acknowledged completions, consistent row and worktree status, and preservation of raw done state. The PR maps unvisited done agents to…
Out of Scope Changes check ✅ Passed The source changes and tests support the acknowledgement-aware completion presentation required by #20940. The changes do not implement the separate SSH row-retention behavior from #10130. Documentati…
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 12 files.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one scope question and one coverage nit.

Reviewed changes

  • New worktreeAgentDisplayState (worktree-card-agent-display-state.ts) — interrupted wins first, non-done delegates to agentRowDotState, and done folds through the shared dashboardCardDisplayState, so an unvisited completion stays done and an acknowledged one settles to idle.
  • Display state threaded through the sidebar — WorktreeCardAgents derives displayStateByPaneKey from the existing unvisitedByPaneKey memo and passes it to full rows, compact rows, and the compact summary.
  • Optional display map on the summary builders — summarizeAgents, buildSummaryAgentGroups, and summarizeAgentIdentities accept a per-pane map and fall back to raw dot state when absent.
  • DashboardAgentRow gains an optional displayState that overrides the leading dot and the no-prompt label fallback while leaving agent.state untouched; no dashboard call site passes it.
  • New unit test covering unvisited-done, acknowledged-done (raw state unchanged), and interrupted preservation.

ℹ️ Worktree-level status dot can still read Done for the same acknowledged completion

The change settles the inline agent row to idle, but the worktree card's own status indicator is derived from hasLiveDone/hasRetainedDone and ignores the acknowledgement map. A worktree whose only agent has been acknowledged will therefore show Idle in the agent row while its card status dot stays green Done — a contradiction that did not exist before this PR, when both read Done.

Technical details
# Worktree status lane vs. per-row display state

## Affected sites
- `src/renderer/src/components/sidebar/worktree-agent-activity-summary.ts:245` — `applyLiveAgentState` sets `hasLiveDone` for any fresh `done` entry, with no acknowledgement input.
- `src/renderer/src/lib/worktree-status.ts:218` — `hasLiveDone || hasRetainedDone` resolves the worktree status to `done`.
- `src/renderer/src/components/sidebar/WorktreeCardStatusSlot.tsx:144` — renders that status in the card's leading lane, directly above the inline agent list whose row now reads idle.

## Required outcome
- Decide whether the worktree status lane should follow the per-row settlement to `idle`, or stay `done` until the worktree is marked read.

## Open questions for the human
- Is the worktree-level `done` intentionally independent of the per-pane acknowledgement? The dashboard's equivalent bucket already folds acknowledged completions to `idle`, so the lane is now the only surface that keeps saying `Done`.

ℹ️ Nitpicks

  • The only test for the new behavior is the pure-function unit test; WorktreeCardAgents.test.tsx mocks DashboardAgentRow, so nothing exercises the displayStateByPaneKey plumbing through full rows, compact rows, or the compact summary. An integration assertion that an acknowledged done row renders idle (and an unvisited one renders done) would pin the wiring, not just the fold.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@peeyush-ciridae

Copy link
Copy Markdown
Author

Addressed the automated review feedback in aaf51f19:

  • The worktree-level status now applies the same acknowledgement boundary as its agent rows. Live and retained completions contribute Done only while unacknowledged, so the card can no longer contradict an acknowledged Idle row.
  • Added rendered integration coverage for full rows, compact rows, compact summaries, live completions, and retained completions.
  • Added documentation to the touched display-state functions/components.
  • Rewrote the PR description using the repository template, linked the related lifecycle issue with an explicit scope note, and documented testing, platform impact, AI use, and the skill-sharing boundary.

Validation: 6 focused test files / 62 tests passed, renderer typecheck passed, changed-code quality reported zero new findings, and formatting passed.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This run re-reviewed the one commit added since the prior pullfrog review (aaf51f1), which resolves both points from that review.

  • Worktree completion flags are now acknowledgement-aware — hasLiveDone and hasRetainedDone are only set while acknowledgedAt < stateStartedAt, so an acknowledged completion settles the worktree status lane instead of leaving it green done above an idle row.
  • Acknowledgement wired into the summary cache — acknowledgedAgentsByPaneKey joins AgentActivityInput, the module-level cache key, and WorktreeActivityStatusState, so an acknowledgement invalidates and rebuilds the cached index while structurally equal summaries keep their references.
  • Integration coverage added — rendered tests pin the full-row display-state map, the acknowledged compact row/summary labels, and the live and retained worktree-status transitions, closing the prior coverage gap.

Verified: the acknowledgement map is replaced immutably, so the cache identity check genuinely fires; stateStartedAt is always a real clock for live and retained rows, so the strict < matches the dashboard's unseen; and interrupted/working rows stay ungated. Focused suite (6 files, 62 tests) passes locally.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@peeyush-ciridae

Copy link
Copy Markdown
Author

Final review cleanup is in 543642ac:

The prior validation remains green: 6 focused files / 62 tests, renderer typecheck, changed-code quality, and formatting.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Acknowledged agent completion remains Done in sidebar

1 participant