Skip to content

fix(workbench): 🐛 Restore sidebar thread statuses#207

Merged
jorben merged 2 commits into
masterfrom
fix/restart-thread-state
May 27, 2026
Merged

fix(workbench): 🐛 Restore sidebar thread statuses#207
jorben merged 2 commits into
masterfrom
fix/restart-thread-state

Conversation

@jorben
Copy link
Copy Markdown
Contributor

@jorben jorben commented May 27, 2026

Summary

  • Restore sidebar thread status records during workspace sidebar sync from thread list snapshots.
  • Keep waiting approval, needs reply, and failed indicators correct after app restart before opening a thread.
  • Add unit coverage for snapshot status initialization and idle snapshot downgrade protection.

Test Plan

  • npm run test:unit -- src/modules/workbench-shell/model/thread-store.test.ts
  • npm run typecheck

🤖 Generated with TiyCode

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

AI Code Review Summary

PR: #207 (fix(workbench): 🐛 Restore sidebar thread statuses)
Preferred language: English

Overall Assessment

No blocking issue was detected in the reviewed diff; keep focused regression testing before merge.

Major Findings by Severity

No major issues identified from the reviewed diff.

Actionable Suggestions

  • Verify that backendToThreadRunStatus gracefully handles undefined/null status values from the backend, either via its implementation or a type-level guarantee on the backend thread shape.

Potential Risks

  • If backend thread statuses are occasionally null/undefined and backendToThreadRunStatus doesn't defensively handle that, snapshot sync could inject unexpected status values into the store.

Test Suggestions

  • A unit test for backendToThreadRunStatus with null/undefined input would add confidence, though this depends on that function's contract being visible.

File-Level Coverage Notes

  • src/modules/workbench-shell/model/workbench-actions.ts: ok (The integration into performSidebarSync is well-placed — after threadsByWorkspaceId is built but before the rest of the sync continues. The Object.keys guard at R838 is a minor optimization; batchSetThreadStatuses({}) would be a no-op regardless. The source:'snapshot' / runId:null pattern is consistent and clean.)
  • src/modules/workbench-shell/model/thread-store.test.ts: ok (Two new tests cover the key behaviors: snapshot initialization and the idle-snapshot-does-not-overwrite-active-stream guard. The test at R253-R268 is especially important as it validates the priority rule that prevents sidebar sync from regressing live run status. Good coverage.)

Inline Downgraded Items (processed but not inline)

  • None

Coverage Status

  • Target files: 2
  • Covered files: 2
  • Uncovered files: 0
  • No-patch/binary covered as file-level: 0
  • Findings with unknown confidence (N/A): 0

Uncovered list:

  • None

No-patch covered list:

  • None

Runtime/Budget

  • Rounds used: 1/4
  • Planned batches: 1
  • Executed batches: 1
  • Sub-agent runs: 1
  • Planner calls: 1
  • Reviewer calls: 1
  • Model calls: 2/64
  • Structured-output summary-only degradation: NO

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Automated PR review completed.

  • Findings kept: 0
  • Findings with unknown confidence: 0
  • Inline comments attempted: 1
  • Target files: 2
  • Covered files: 2
  • Uncovered files: 0
    See the summary comment for detailed analysis and coverage details.

@@ -224,6 +224,49 @@ describe("batchSetThreadStatuses", () => {
expect(statuses["thread-2"].status).toBe("idle");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated review completed for this PR diff. No concrete inline issue was selected after aggregation.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Automated PR review completed.

  • Findings kept: 0
  • Findings with unknown confidence: 0
  • Inline comments attempted: 1
  • Target files: 2
  • Covered files: 2
  • Uncovered files: 0
    See the summary comment for detailed analysis and coverage details.

@@ -224,6 +224,49 @@ describe("batchSetThreadStatuses", () => {
expect(statuses["thread-2"].status).toBe("idle");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated review completed for this PR diff. No concrete inline issue was selected after aggregation.

@jorben jorben merged commit ab5b4c0 into master May 27, 2026
5 checks passed
@jorben jorben deleted the fix/restart-thread-state branch May 27, 2026 14:57
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.

1 participant