Skip to content

server, docs: hold auto-settle while a background command runs - #17626

Closed
dominic-r wants to merge 1 commit into
pingdotgg:mainfrom
d5cr:fix/auto-settle-background-command
Closed

dominic-r wants to merge 1 commit into
pingdotgg:mainfrom
d5cr:fix/auto-settle-background-command

Conversation

@dominic-r

@dominic-r dominic-r commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #17625

Related: #15099 (the completion alert for the same case, which this PR leaves unchanged) and #15413.

Problem

A thread auto-settles while the agent is still waiting on a live background command. Claude records every background Bash as a pending task of kind command, and the CLI wakes the agent when that command exits. Auto-settlement reused the completion-alert rule, backgroundWorkHoldsCompletion, which treats a command as "the agent is done". When the merge rule or the inactivity rule came due, the thread settled while the composer still showed the command running, and the agent later woke up in a settled thread.

Change

isAutoSettlementCandidate now rejects a thread with any live pending background task, whatever its kind. That one check covers both the settlement sweep and resolveAutoSettlementAt, so the merge rule and the inactivity rule both wait. Completion alerts and sidebar status keep their current rule, so a dev server left running still lets the "done" notification fire.

The user guide's auto-settle section now says a background command holds the thread until it exits.

Trade-off: a thread with a dev server or another long-lived command left running stays unsettled until the command exits or the user settles the thread by hand. Auto-settle is silent and hides the thread, so holding it a little too long is the cheaper mistake.

Scope and approval

This fixes the bug reported in #17625, using the fix that issue suggests. It changes only the auto-settlement gate on the server, plus its tests and one sentence of user docs.

Verification

Replaced the test that expected a live command to allow auto-settle with one that expects it to hold the thread. Added a test where a thread with a live command does not settle under either the merged-PR rule or the inactivity rule, and the same thread with no pending tasks settles under both. Both new tests fail with the old check and pass with the fix. The existing settlement service and settlement projection tests still pass, and the server package typechecks. I did not reproduce the bug end to end in a running app.

Written by Claude Opus 5.5 via t3code. This PR was opened by Grok Bot.

Auto-settlement reused the completion-alert rule, which ignores pending
background commands. A Claude background Bash the agent is waiting on
wakes the agent when it exits, so the thread settled while it was still
working. Any live pending background task now holds auto-settle. Alerts
and sidebar status keep their rule.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 9, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at ab35920

Macroscope's review found this PR approvable — This is a narrowly scoped, well-tested bug fix that prevents auto-settlement while any background task can still wake the agent, without changing configured default values or unrelated runtime behavior. The documentation accurately describes the intentional delay for long-lived commands.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dcb531cb-4733-4c57-b694-747abf6cf914

📥 Commits

Reviewing files that changed from the base of the PR and between b1ec4b3 and ab35920.


📒 Files selected for processing (3)
  • apps/server/src/orchestration-v2/ThreadSettlementService.test.ts
  • apps/server/src/orchestration-v2/ThreadSettlementService.ts
  • docs/user/thread-sidebar.md

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



📝 Walkthrough

Walkthrough

Automatic settlement now excludes a thread while any background task is pending. Tests cover merge-based and inactivity-based settlement. Sidebar documentation describes that a running background command blocks automatic settlement until it exits or the thread is settled manually.

Changes

Auto-settlement

Layer / File(s) Summary
Pending-task settlement gate
apps/server/src/orchestration-v2/ThreadSettlementService.ts, apps/server/src/orchestration-v2/ThreadSettlementService.test.ts, docs/user/thread-sidebar.md
The candidate check excludes threads with any pending background task. Tests verify that a pending command blocks merge and inactivity settlement, and that both rules can settle after the task is removed. The sidebar documentation describes this behavior.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg


Merge Risk: ⚪ Minimal · up to ab359

Threads with a running background command stay unsettled until the command exits or the user settles them manually. The PR describes this as intended, and the docs are updated. No merge-blocking risk was found.

Architecture Summary

Architecture risk: 🔵 Low · up to ab359

The change affects 2 systems.

Changed systems: apps/server, docs

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 2 changed files map to changed impact.
  • observed — docs (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/orchestration-v2/ThreadSettlementService.test.ts: Replaces the test expecting a thread with a running command to remain an auto-settlement candidate with one expecting it to be excluded; the new fixture uses a CI watch command and adds a comment that the command may wake a waiting agent when it exits.
  • observed — Modified behavior in apps/server/src/orchestration-v2/ThreadSettlementService.test.ts: Adds coverage that a pending background command prevents both merge-triggered and inactivity-triggered settlement, and that both rules settle at the last run completion time once the command is no longer pending.
  • observed — Modified behavior in apps/server/src/orchestration-v2/ThreadSettlementService.ts: Removed the backgroundWorkHoldsCompletion import.
  • observed — Modified behavior in apps/server/src/orchestration-v2/ThreadSettlementService.ts: Updated the live-work explanation and replaced the completion-hold predicate with a check that excludes a thread whenever its pending background-task list is nonempty. Previously, only background work classified as holding completion blocked settlement.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue #17625 requires auto-settlement to wait for live pending background work under both merged-PR and inactivity rules. isAutoSettlementCandidate now rejects any non-empty pendingBackgroundTasks…
Out of Scope Changes check Passed The changes stay within issue #17625. The server change updates only the auto-settlement gate. The tests cover the required command and settlement-rule behavior. The documentation explains the resulti…
Title check Passed The title clearly identifies the server and documentation change: auto-settlement now waits while a background command runs.
Description check Passed The description covers the problem, change, scope, trade-off, verification results, and related issues. It also states the unverified end-to-end check. The focused-fix rationale adequately addresses t…


✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one does not say which model or harness was used to create it. If this change is really important, we recommend rebuilding the PR with a newer model and noting the model and harness in the PR description.

@dominic-r

Copy link
Copy Markdown
Contributor Author

Reopened as #18062 with the model and harness noted: Claude Opus 5.5 via t3code.

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

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

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Thread auto-settles while the agent is still waiting on a background command

2 participants