Skip to content

fix(server): deleted threads no longer keep their settle-action record - #28954

Closed
only21mil wants to merge 4 commits into
pingdotgg:mainfrom
only21mil:t3/delete-settle-actions-on-thread-delete
Closed

only21mil wants to merge 4 commits into
pingdotgg:mainfrom
only21mil:t3/delete-settle-actions-on-thread-delete

Conversation

@only21mil

@only21mil only21mil commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Problem

ThreadSettlementService remembers the settledAt of every thread whose settle action ran, so a re-emitted thread.settled does not run the action twice. Nothing ever removes an entry. Deleting a thread ends its settlements for good (later commands reject a deleted ID, and creation won't reuse it), but its entry stays in memory until the server restarts. On a long-running server, every thread that reaches the settle-action step (a settled thread with an existing worktree, whatever the script outcome) and is then deleted leaves one entry behind.

Change

On thread.deleted, the service now also removes that thread's entry from settleActionRunAt, then queues the full sweep it already queued. Nothing else changes:

  • Live threads keep the same dedupe. A repeated settlement with the same timestamp still runs the action once. A failed start still retries. A re-engaged thread that settles at a new time still runs again.
  • The entry is still kept when a thread re-engages, because the event consumer can lag the projection. Clearing it on unsettle could start a newer settlement's action twice.
  • shellClosedThreads, retry policy, the forced PR refresh and the persisted state are untouched. Clearing shellClosedThreads on delete would not bound it, since a late command completion can add the ID again. That is left out on purpose.

Scope and approval

There is no issue. I think this fits CONTRIBUTING's exception for a very small, focused fix of an obvious bug. It frees memory that a terminal state made unreachable, and it changes no product behavior or settings, so it needs no direction call. If maintainers would rather not take housekeeping at this size, closing it is fine.

Nothing on screen changes. The map is private to the server process and only read by the settle-action dedupe check. No projection, event, RPC or contract is touched.

Verification

  • vp test run src/orchestration-v2/ThreadSettlementService.test.ts: the extended case "runs the settle script once per settlement until the thread is deleted" fails on main (the replay after delete is still suppressed) and passes with the fix. In the same case, deleting a different thread leaves the live thread's dedupe in place. Again on 2026-11-12, on 3f6c200 merged locally with main dbfbe3993f (not pushed): with ThreadSettlementService.ts from that main the case fails (expected 'marker-thread' to equal 'repeated-thread', 1 failed, 37 passed), and with this PR's version all 38 pass.
  • At 3f6c200, vp run check:changed 836b86a5d6 passes (format, lint, knip, graphql, typecheck, tests), and the three settlement suites pass 53 tests.
  • Real run: a T3 server from source on main 836b86a and on 3f6c200, with the free Replay (dev) provider. Two worktree threads with a project settle action both settled, thread A was deleted, then B settled again. I read settleActionRunAt through the inspector at breakpoints. On main the deleted thread's entry stays; on 3f6c200 the delete removes it and then queues the full sweep, also when another settlement's sweep is already pending.
Inspector readback (selected lines, copied without changes)

Real server, main 836b86a:

[2026-11-11T21:11:39.854Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":false,"outcome":"enqueues a full sweep","settleActionRunAt.entries":[["a81b4ccd-66c0-42f2-b3db-487b5cc984c9",1794431496970],["e408f945-ca67-4062-803b-162d6f2b6329",1794431497164]]}
[2026-11-11T21:11:44.518Z] RESULT: deleted Thread A (a81b4ccd-66c0-42f2-b3db-487b5cc984c9) still a key of settleActionRunAt after its thread.deleted: true

Real server, 3f6c200:

[2026-11-11T21:11:08.991Z] inspector paused at deletion cleanup (inside Effect.sync, before Map.delete) (ThreadSettlementService.ts:683), evaluateOnCallFrame read: {"at":"deletion cleanup","event":"thread.deleted","threadId":"eaa32832-8cbd-4bd8-a3a2-05bec5093feb","settleActionRunAt.entries before delete":[["eaa32832-8cbd-4bd8-a3a2-05bec5093feb",1794431465915],["4947ab54-cd55-41b8-ab75-e3b861972234",1794431466119]]}
[2026-11-11T21:11:09.076Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":false,"outcome":"enqueues a full sweep","settleActionRunAt.entries":[["4947ab54-cd55-41b8-ab75-e3b861972234",1794431466119]]}
[2026-11-11T21:11:09.159Z] inspector paused at full sweep worker start (ThreadSettlementService.ts:589), evaluateOnCallFrame read: {"at":"full sweep worker starts runSweep(null)","fullSweepPending":true}
[2026-11-11T21:11:14.116Z] RESULT: deleted Thread A (eaa32832-8cbd-4bd8-a3a2-05bec5093feb) still a key of settleActionRunAt after its thread.deleted: false

Real server, 3f6c200, B's settlement and A's delete sent together:

[2026-11-11T21:14:48.761Z] RACE: dispatching thread.settle Thread B and thread.delete Thread A concurrently
[2026-11-11T21:14:48.927Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":false,"outcome":"enqueues a full sweep","settleActionRunAt.entries":[["6ced7843-854c-4a21-bb8c-ec93ad41595a",1794431686069],["c1160e97-d96f-482c-b9b0-d8448ea4d3f7",1794431688786]]}
[2026-11-11T21:14:49.010Z] inspector paused at deletion cleanup (inside Effect.sync, before Map.delete) (ThreadSettlementService.ts:683), evaluateOnCallFrame read: {"at":"deletion cleanup","event":"thread.deleted","threadId":"6ced7843-854c-4a21-bb8c-ec93ad41595a","settleActionRunAt.entries before delete":[["6ced7843-854c-4a21-bb8c-ec93ad41595a",1794431686069],["c1160e97-d96f-482c-b9b0-d8448ea4d3f7",1794431688786]]}
[2026-11-11T21:14:49.093Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":true,"outcome":"coalesced into the pending full sweep","settleActionRunAt.entries":[["c1160e97-d96f-482c-b9b0-d8448ea4d3f7",1794431688786]]}
[2026-11-11T21:14:49.176Z] inspector paused at full sweep worker start (ThreadSettlementService.ts:589), evaluateOnCallFrame read: {"at":"full sweep worker starts runSweep(null)","fullSweepPending":true}
  • One-off memory probe, not committed: the real ThreadSettlementService layer with the test file's mocked dependencies settled, deleted and replayed 20,000 synthetic threads. main still held 20,000 entries after the deletes; this PR held 0 and released 1,230,584 B (about 62 B per thread). Re-engaged and lagging settlements behaved the same on both builds.

Not checked:

  • Real-world memory on a production server. The probe numbers are synthetic and only show the per-entry cost. I make no claim about user-visible impact.
  • A delete that arrives while a full sweep is already running, other providers, and the client apps.

Prior PRs

The record and its dedupe came from #16290 by t3dotgg, which built on #15991 by spoukyii. #26291 by Wosca changes the settle-action step without removing these entries. #19665 by mikeohuo, merged as 20081ef, made thread.deleted queue a full sweep; at 3f6c200 the same case also removes the entry.

Generated with Claude Opus 5.5 and GPT-6.1 Terra and GPT-6.1 Sol in T3 Code Nightly

only21mil and others added 2 commits November 1, 2026 23:08
The settlement service remembers the settledAt of each thread whose settle
action ran, so a re-emitted thread.settled does not run it twice. Nothing
removed that record, so every settled-then-deleted thread kept one for the
life of the server process. Drop it when thread.deleted is consumed; live
threads keep their per-settlement dedupe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e test

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched Author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 1, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 6d36ce6

Macroscope's review found this PR approvable — Deleted threads now release their in-memory settle-action record. The cleanup is confined to private bookkeeping, leaving live-thread deduplication and persistent data unchanged.

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

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

ThreadSettlementService now clears a thread’s settlement-run marker when it processes a deletion event. Tests cover repeated settlement events before and after deletion, including deletion of an unrelated thread.

Changes: Thread settlement deletion handling

Layer / File(s) Summary
Clear settlement marker on thread deletion
apps/server/src/orchestration-v2/ThreadSettlementService.ts, apps/server/src/orchestration-v2/ThreadSettlementService.test.ts
The deletion handler removes the deleted thread’s settlement-run marker before enqueueing a full sweep. Tests verify that deleting an unrelated thread does not clear the marker, and that deleting the settled thread allows its script to run again.

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

Change: Bug fix

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 3f6c2

Deleted threads no longer keep their settle-action record in server memory, and live-thread behavior is unchanged. No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: removing settle-action records for deleted threads.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description follows the repository template closely. It has Problem, Change, Scope, and Verification sections, and each section gives specific content. The Problem section states the leak: settleA…
Full details: Description check

Explanation

The description follows the repository template closely. It has Problem, Change, Scope, and Verification sections, and each section gives specific content. The Problem section states the leak: settleActionRunAt entries are never removed, so each deleted settled thread leaves one entry until restart. The Change section describes the fix, the dedupe behavior that stays the same, and what was left out on purpose. The Scope section explains why no issue is linked and argues for the small-fix exception in CONTRIBUTING. It does not link an issue, which the template asks for. The Verification section lists focused test runs, a check:changed run, a real-server run with inspector readbacks, and a synthetic memory probe. It also states what was not checked. The template asks for the model and harness used, and the description includes that. The description has some extra material in its Prior PRs section, but that does not hurt the structure. The main gap is the missing issue link or a stronger case for the exception. The Scope section does make that case, so the gap is small.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 6d36ce6 and 3f6c200.

Files selected for processing: 2

  • apps/server/src/orchestration-v2/ThreadSettlementService.test.ts
  • apps/server/src/orchestration-v2/ThreadSettlementService.ts

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

@maria-rcks

maria-rcks commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified as-is at 6d36ce6f015183bf61376d17ebba9096f10f0d0e on a disposable linux vm, using repository playwright and /usr/bin/google-chrome against the real dev web app.

  • reproduced on github/main (f6503f0e71) in a separate worktree: created a real git project through the app, configured a settle action through the action editor, completed a real codex gpt-6.1-sol worktree turn, settled the thread, then deleted it and its worktree. the private settle-action map still retained the deleted thread id.
  • repeated on this exact pr head with two real codex worktree threads. settling ran the configured shell action once per thread. un-settle followed by settle ran the target's action once more with a new settlement timestamp.
  • deleting the target removed its map entry. the neighboring settled thread's entry and timestamp remained intact. the shell action did not run again during deletion.

observed the existing map in the actual server through node's inspector, capturing its reference at a breakpoint immediately after the normal action bookkeeping. no source edits, mocked providers, synthetic settlement events, or fixture data were used. one dev server ran at a time.

blacksmith-check: the focused settlement suite passed all 31 tests. vp run check:changed fa31b66bbb2ae5234205a8628a78e038d917202f passed format, lint, knip, graphql, affected typechecks, and 32 tests across two files. existing applicable github checks also pass. no code changes were needed.

command transcript and map observations

completed real codex gpt-6.1-sol worktree turn on the pr head

un-settle, settle again, and delete through the real sidebar; idle time removed

Not verified

native ios/android and electron clients, macos/windows shells, other providers, production heap impact, and retry/failure races were not exercised on this linux web-client pass. identical-timestamp event deduplication passed in the focused suite; it was not artificially replayed through the running app because deleted threads cannot settle again through the supported client path. no production memory or performance claim is made.

merge update at 3f6c2009e5700ca37f8638a2a783d2d3f09ba7ad

merged current main (836b86a5d6b7) without rebasing and resolved the test-helper conflict using main's moved helpers. kept main's newer subagent coverage and the contributor's deletion/deduplication assertions. combined deletion handling so it clears the deleted thread's settle-action record and still enqueues main's full settlement sweep. the diff remains two files, 20 additions and 2 deletions.

github reports no conflicts on this head. current-head ci is pending. blacksmith verification is unverified: two task-owned workers reported ready but their github jobs stayed queued and their ssh endpoints were unreachable; dependency installation and the focused suite never started. both workers were stopped. vp run check:changed, typecheck, lint, and tests still need a working blacksmith worker on this head. the earlier runtime evidence above applies to the original head, not this merge update.

@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 Opus 5.5 replying on behalf of Victor.

@maria-rcks I ran the checks this head still needed on 3f6c200, with Node 26.12.1 from the repo root: vp run check:changed 836b86a5d6 passed format, lint, knip, graphql, typecheck (3 packages) and 40 tests in 2 files, and the settlement suites passed 53 tests (ThreadSettlementService 38, ProjectionSettlement 6, SubagentProjection 9). With ThreadSettlementService.ts put back to d7200b6, the extended dedupe test fails (1 failed, 37 passed), and with 3f6c200's version all 38 pass. I also ran a real T3 server from source on 3f6c200 and on main 836b86a with the free Replay (dev) provider: two worktree threads with a project settle action, both settled (the action ran for each), thread A deleted, then B settled again, reading settleActionRunAt through the inspector at breakpoints. On main, A's entry stays after the delete; on 3f6c200 the delete removes it and then queues the full sweep, and when the delete lands while another settlement's full sweep is pending, it removes the entry and joins that pending sweep, so one sweep runs. Not checked: a delete that arrives while a full sweep is already running, other providers, and the client apps.

Output (selected lines, copied without changes)

vp run check:changed 836b86a5d6 on 3f6c200:

check:changed: 2 files since 836b86a5d6b7 (836b86a5d6)
check:changed summary
  pass  format  0.2s
  pass  lint  0.5s
  pass  knip 1/2  7.5s
  pass  knip 2/2  13.3s
  pass  graphql (apps/server)  0.4s
  pass  typecheck (3 packages)  9.2s
  pass  test (2 files)  12.6s

Settlement suites on 3f6c200:

 Test Files  3 passed (3)
      Tests  53 passed (53)

Extended dedupe test, ThreadSettlementService.ts from d7200b6, then from 3f6c200:

     × runs the settle script once per settlement until the thread is deleted 5ms
      Tests  1 failed | 37 passed (38)
      Tests  38 passed (38)

Real server, main 836b86a:

[2026-11-11T21:11:39.854Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":false,"outcome":"enqueues a full sweep","settleActionRunAt.entries":[["a81b4ccd-66c0-42f2-b3db-487b5cc984c9",1794431496970],["e408f945-ca67-4062-803b-162d6f2b6329",1794431497164]]}
[2026-11-11T21:11:44.518Z] RESULT: deleted Thread A (a81b4ccd-66c0-42f2-b3db-487b5cc984c9) still a key of settleActionRunAt after its thread.deleted: true

Real server, 3f6c200:

[2026-11-11T21:11:08.991Z] inspector paused at deletion cleanup (inside Effect.sync, before Map.delete) (ThreadSettlementService.ts:683), evaluateOnCallFrame read: {"at":"deletion cleanup","event":"thread.deleted","threadId":"eaa32832-8cbd-4bd8-a3a2-05bec5093feb","settleActionRunAt.entries before delete":[["eaa32832-8cbd-4bd8-a3a2-05bec5093feb",1794431465915],["4947ab54-cd55-41b8-ab75-e3b861972234",1794431466119]]}
[2026-11-11T21:11:09.076Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":false,"outcome":"enqueues a full sweep","settleActionRunAt.entries":[["4947ab54-cd55-41b8-ab75-e3b861972234",1794431466119]]}
[2026-11-11T21:11:09.159Z] inspector paused at full sweep worker start (ThreadSettlementService.ts:589), evaluateOnCallFrame read: {"at":"full sweep worker starts runSweep(null)","fullSweepPending":true}
[2026-11-11T21:11:14.116Z] RESULT: deleted Thread A (eaa32832-8cbd-4bd8-a3a2-05bec5093feb) still a key of settleActionRunAt after its thread.deleted: false

Real server, 3f6c200, B's settlement and A's delete sent together:

[2026-11-11T21:14:48.761Z] RACE: dispatching thread.settle Thread B and thread.delete Thread A concurrently
[2026-11-11T21:14:48.927Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":false,"outcome":"enqueues a full sweep","settleActionRunAt.entries":[["6ced7843-854c-4a21-bb8c-ec93ad41595a",1794431686069],["c1160e97-d96f-482c-b9b0-d8448ea4d3f7",1794431688786]]}
[2026-11-11T21:14:49.010Z] inspector paused at deletion cleanup (inside Effect.sync, before Map.delete) (ThreadSettlementService.ts:683), evaluateOnCallFrame read: {"at":"deletion cleanup","event":"thread.deleted","threadId":"6ced7843-854c-4a21-bb8c-ec93ad41595a","settleActionRunAt.entries before delete":[["6ced7843-854c-4a21-bb8c-ec93ad41595a",1794431686069],["c1160e97-d96f-482c-b9b0-d8448ea4d3f7",1794431688786]]}
[2026-11-11T21:14:49.093Z] inspector paused at enqueueFullSweep (ThreadSettlementService.ts:594), evaluateOnCallFrame read: {"at":"enqueueFullSweep","fullSweepPending":true,"outcome":"coalesced into the pending full sweep","settleActionRunAt.entries":[["c1160e97-d96f-482c-b9b0-d8448ea4d3f7",1794431688786]]}
[2026-11-11T21:14:49.176Z] inspector paused at full sweep worker start (ThreadSettlementService.ts:589), evaluateOnCallFrame read: {"at":"full sweep worker starts runSweep(null)","fullSweepPending":true}

@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 Opus 5.5 replying on behalf of Victor.

Closing this: it is not a bug fix, and this repository now takes only bug fixes from outside contributors. The branch stays on my fork if it is useful.

@only21mil only21mil closed this Nov 12, 2026
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:unvouched Author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants