Skip to content

fix(server): automatic activity unsettle no longer clears a user's "active" pin - #8015

Open
mattholla wants to merge 1 commit into
pingdotgg:mainfrom
mattholla:fix/preserve-user-unsettle-pin
Open

fix(server): automatic activity unsettle no longer clears a user's "active" pin#8015
mattholla wants to merge 1 commit into
pingdotgg:mainfrom
mattholla:fix/preserve-user-unsettle-pin

Conversation

@mattholla

@mattholla mattholla commented Aug 23, 2026

Copy link
Copy Markdown

What Changed

The decider now emits the automatic activity unsettle only when the override is "settled". Same guard at all three emit sites in decider.ts (turn start, session set, activity append). Two tests asserted the old behavior and now assert the pin survives.

Full apps/server test suite passes with the change: 2642 passed, 0 failed (pnpm 11.10.0, vitest via vite-plus). The change is unit-tested only; I have not run a patched build of the app.

Why

Refs #6417. This addresses the part where an explicit Un-settle doesn't stick. It does not change merged-PR auto-settle itself, so the issue should stay open for a maintainer to judge.

Clicking Un-settle stores settledOverride: "active". At the start of the next turn, the decider emits an automatic thread.unsettled with reason "activity" whenever any override is set, and the reducer maps every non-user reason to null. The pin is erased one turn after it was set, and on a thread with a merged/closed PR settles again right away.

The comments at these emit sites say real activity resets any override, active pins included. I disagree: The pin only matters on a thread that keeps re-settling on its own, and those threads always have activity, so the pin is removed before it can do anything.

Alternative considered: keep "active" in the reducer on non-user reasons. I chose the decider guard instead because 1. It touches one file, while the reducer is copied in three files across client and server. 2. It only affects new events, while a reducer change alters what already-recorded events mean on replay. 3. If #5462 lands, settled state becomes server-authored, so the decider seemed like the right place for the policy either way.

Repro and event-log trace are in my comment on #6417.

Checklist

  • Small and focused
  • Explained what and why
  • UI changes: none (no screenshots needed)
  • Animation/timing changes: none

Note

Medium Risk
Changes orchestration lifecycle policy for thread settle/unsettle. Wrong guards would hide settled threads or drop user pins, but the change is small, localized, and unit-tested.

Overview
Stops automatic thread.unsettled (reason "activity") from wiping a user’s keep-active pin (settledOverride: "active"). Un-settle now survives the next turn, live session, or blocking activity instead of being reset to neutral and immediately auto-settling again.

The decider only prepends that unsettle when the override is "settled", at turn start, session set, and activity append. Tests now assert the pin is left alone.

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

Note

Fix automatic activity unsettle to preserve user's active pin in decideOrchestrationCommand

Previously, any non-null settledOverride triggered a thread.unsettled event, which cleared a user's active pin. The decider now only emits thread.unsettled when settledOverride === 'settled', leaving active-pinned threads intact.

  • Updated unsettle gating in three handlers in decider.ts: thread.turn.start, thread.session.set, and thread.activity.append — all now check for an explicitly settled thread rather than a non-null override.
  • Adjusted test expectations in decider.settled.test.ts to no longer expect thread.unsettled events when a thread has an active pin.
  • Risk: threads pinned active will no longer auto-unsettle on activity, turn start, or session set — callers relying on automatic unsettle for active-pinned threads will see different lifecycle event sequences.

Macroscope summarized defb155.

…ctive" pin

An explicit Un-settle writes settledOverride: "active". Real activity
(a new turn, a session coming alive, an approval/user-input activity)
emits a thread.unsettled event with reason: "activity" to wake a
settled thread, but the reducer treats any non-user reason as a plain
reset to null — so the automatic event was silently clearing a user's
"active" pin back to neutral, the same as if they had never pinned it.

Guard each emit site in decider.ts so the automatic "activity" unsettle
only fires when settledOverride is "settled", never "active". Found a
third such site beyond the two described (thread.activity.append, in
addition to thread.turn.start and thread.session.set) and applied the
same guard there. Reducers are untouched; the fix is entirely in which
events get emitted. Updated decider.settled.test.ts, which asserted
the old behavior (activity clearing an "active" pin) for the turn-start,
session-set, and activity-append paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 85c159fd-2b86-41a3-846d-b2f94f996bb6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

❤️ Share

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

@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 Aug 23, 2026
@macroscopeapp

macroscopeapp Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at defb155

Macroscope's review found this PR approvable — This is a small, localized orchestration bug fix that preserves explicit active pins by suppressing only the automatic unsettle event for already-active overrides. The accompanying tests cover the changed event sequences, and the lockfile metadata change has no runtime impact.

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

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.

1 participant