Skip to content

fix(server): skip disabled settlement lookups - #10313

Closed
yashranaway wants to merge 2 commits into
pingdotgg:mainfrom
yashranaway:fix/skip-disabled-settlement
Closed

fix(server): skip disabled settlement lookups#10313
yashranaway wants to merge 2 commits into
pingdotgg:mainfrom
yashranaway:fix/skip-disabled-settlement

Conversation

@yashranaway

@yashranaway yashranaway commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

What Changed

Check settlement settings before reading the shell snapshot or preparing candidates. Disabled timer and merge sweeps perform no snapshot or pull-request lookup work.

The final decision still reads fresh settings so disabling settlement during a lookup prevents a stale decision. Re-enabling the setting resumes settlement.

Why

Automatic settlement looks up candidate pull requests even when both settings are disabled, spawning Git and provider processes on each scheduled sweep.

Fixes #9714.

Testing

  • ThreadSettlementReactor.test.ts passes, 12/12, covering disabled timer and merge sweeps and re-enabling settlement.
  • Server typecheck, targeted lint, and formatting pass.

Checklist

  • One focused change
  • Explained what changed and why
  • Added regression coverage for the changed behavior
  • No UI layout or animation changes

Model: GPT-6 Astra
Harness: T3 code

Note

Skip settlement work in ThreadSettlementReactor.sweep when both triggers disabled

  • The sweep method now reads server settings before loading the shell snapshot and returns immediately when merge-based settlement is disabled and the day-based interval is unset
  • Test harness makeThreadSettlementHarness records settings-service reads so tests can observe and assert on settings access
  • Adds a test verifying that startup, activation, timer, and merge sweeps perform no snapshot, pull-request, invalidation, or command work while both automatic-settlement settings are disabled, then resume after one is enabled
  • Behavioral Change: a disabled sweep now exits before the snapshot load, so existing tests that expected snapshot or lookup work during disabled sweeps have updated count expectations in ThreadSettlementReactor.test.ts

Macroscope summarized 68a4096.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 6, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 68a4096

Macroscope's review found this PR approvable — This is a focused server bug fix that avoids snapshot, Git, and pull-request lookups when both existing settlement options are disabled, while preserving enabled behavior. Regression tests cover startup, timer, merge, and re-enabling scenarios, and product defaults remain unchanged.

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

@t3dotgg

t3dotgg commented Sep 6, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Fable 5.1 responding on behalf of Theo

Rebased onto main as #10424 so this can land for the stable release. #10103 changed the same sweep function and test file, and my token could not push the rebase to your fork. Your commit and authorship are preserved. Only change: the rebased test asserts on dispatched commands instead of lookup counts, because inactive threads now settle without a PR lookup. Closing this in favor of #10424.

@t3dotgg t3dotgg closed this Sep 6, 2026
@yashranaway
yashranaway deleted the fix/skip-disabled-settlement branch September 6, 2026 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] ThreadSettlementReactor spawns ~50 git/min even when auto-settle is disabled; vcs.refreshStatus sits at 10s p50

2 participants