fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR - #4309

Merged
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message
Jul 23, 2026
Merged

fix(client-runtime): keep a warm thread un-settled despite a merged/closed PR#4309
t3dotgg merged 3 commits into
mainfrom
t3code/unsettle-thread-on-message

Conversation

@t3dotgg

@t3dotggt3dotgg commented Jul 22, 2026

Copy link
Copy Markdown
Member

Problem

Sending a message in a settled thread should un-settle it. Two of the three settle signals already behaved:

  • Explicit override — the server decider prepends thread.unsettled when thread.turn.start hits an overridden thread (decider.ts).
  • Inactivity auto-settle — the new message refreshes threadLastActivityAt.

But the merged/closed-PR auto-settle signal in effectiveSettled is client-derived and never clears. Send a message in a merged-PR thread and the row un-settles only while the queued-turn grace / live-session blockers hold — the moment the turn completes, changeRequestState === "merged" classifies it right back into the settled tail. This also defeats the server's un-settle on explicitly settled threads whose PR is merged: the override clears on send, then the merge signal re-settles the row anyway.

Fix

Gate the change-request signal on idleness: a merged/closed PR settles a thread only after it has been quiet for CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour). Fresh activity — the sent message, its turn starting and completing — keeps the follow-up conversation in the active list; once the burst goes stale, the merge signal settles it again. This mirrors the decider's own "activity clears overrides so the thread can auto-settle again after this burst of work goes stale" model.

Web (SidebarV2.tsx) and mobile (threadListV2.ts) both partition through the shared effectiveSettled, so one change covers both clients. No contract or server changes: the VCS status contract carries no merge timestamp, so idle-based gating is the clean option.

The explicit Settle action on a warm merged-PR thread still works — a user settle sets settledOverride, which is checked before the merge signal.

Testing

  • New test: a warm thread (recent activity) stays active under both merged and closed PR states; an idle one still settles.
  • Full client-runtime suite passes (449 tests), mobile threadListV2 tests pass, lint and typecheck clean.

🤖 Generated with Claude Code


Note

Low Risk
Client-only list-partitioning logic in shared effectiveSettled; behavior is narrower (fewer surprise re-settles) with targeted tests and no server/contract changes.

Overview
Fixes merged/closed PR threads snapping back to settled right after a follow-up message’s turn finishes. effectiveSettled used to treat merged/closed change requests as an always-on settle signal, so activity blockers only briefly kept the row active.

Merged/closed PRs now auto-settle only when threadLastActivityAt is missing or older than CHANGE_REQUEST_SETTLE_IDLE_MS (1 hour) relative to now. Recent follow-ups stay in the active list; after the burst goes idle, the merge signal settles the thread again. Explicit settled / active overrides and existing activity blockers are unchanged.

Tests cover warm vs idle boundaries for merged and closed, and re-settling once the idle window passes.

Reviewed by Cursor Bugbot for commit 3cea3f9. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Keep warm threads active despite merged/closed PR until idle for one hour

  • Previously, effectiveSettled would auto-settle a thread as soon as its change request reached merged or closed state, even if the thread had recent activity.
  • Adds a CHANGE_REQUEST_SETTLE_IDLE_MS constant (1 hour) in threadSettled.ts; a thread now only auto-settles if lastActivityAt is older than now - 1h (or absent).
  • Behavioral Change: threads with a merged/closed PR that receive post-merge activity will remain unsettled for up to one hour after the last activity.

Macroscope summarized 3cea3f9.

Summary by CodeRabbit

  • Bug Fixes
    • Threads associated with merged or closed change requests now remain active during follow-up activity.
    • Threads automatically settle after at least one hour of inactivity, preventing premature status changes.

…losed PR
Sending a message in a settled thread un-settles it server-side (the
decider prepends thread.unsettled on turn.start), but the merged/closed-PR
auto-settle signal in effectiveSettled is client-derived and never clears,
so the row snapped back into the settled tail the moment the new turn
completed.
Gate the change-request signal on thread idleness: a merged/closed PR only
settles a thread quiet for at least an hour. Fresh activity keeps the
follow-up conversation in the active list; once the burst goes stale the
merge signal settles it again, matching the decider's activity-clears-
overrides model. Web and mobile both partition through the shared
effectiveSettled, so this covers both clients.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: client-runtime settlement now stays active for warm merged/closed PR threads.
Description check✅ PassedThe description explains what changed, why, and testing, but it does not follow the repo's requested section headings or checklist format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3code/unsettle-thread-on-message

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

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@macroscopeapp

macroscopeappBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This is a targeted bug fix for thread settling logic on merged/closed PRs. The change adds an idle guard to prevent threads from immediately re-settling after follow-up activity. The author owns this code, the scope is limited, and comprehensive tests are included.

You can customize Macroscope's approvability policy. Learn more.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026
…le; note clock-skew exposure
Codex review follow-ups: assert the strict one-hour boundary (activity
exactly an hour old stays warm), assert the same shell re-settles once the
follow-up burst goes idle, and document that cross-device clock skew
shifts the idle window by its size — the same accepted exposure as the
inactivity auto-settle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 22, 2026 21:14

Dismissing prior approval to re-evaluate ca5b817

macroscopeapp[bot]
macroscopeappBot previously approved these changes Jul 22, 2026
@github-actionsgithub-actionsBot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 22, 2026

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/client-runtime/src/state/threadSettled.ts`:
- Around line 107-114: Update the lifecycle documentation comment above the
settled-resolution logic to clarify that pending work and unadjudicated queued
turns are evaluated before explicit settled or unsettled overrides. Replace the
claim that overrides “win in both directions” with wording that accurately
reflects these blockers taking precedence, while preserving the existing
override behavior after blockers clear.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b1ae8f2c-80fb-4fb9-aeec-55962715db89

📥 Commits

Reviewing files that changed from the base of the PR and between 9a0a071 and ca5b817.

📒 Files selected for processing (2)
  • packages/client-runtime/src/state/threadSettled.test.ts
  • packages/client-runtime/src/state/threadSettled.ts

Comment threadpackages/client-runtime/src/state/threadSettled.ts Outdated
… override
CodeRabbit review: the effectiveSettled doc claimed the override "wins in
both directions", but pending approvals/user-input, live sessions, and
unadjudicated queued turns are evaluated first and hold a thread active
regardless of any override. Say so explicitly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@macroscopeapp
macroscopeappBot dismissed their stale reviewJuly 23, 2026 20:06

Dismissing prior approval to re-evaluate 3cea3f9

@t3dotgg
t3dotgg merged commit 193e3c6 into mainJul 23, 2026
@t3dotgg
t3dotgg deleted the t3code/unsettle-thread-on-message branch July 23, 2026 22:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M30-99 changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@t3dotgg@maria-rcks