fix(server): settle inactive threads with persisted open PR links

[aravhawk]

Problem

An idle thread with a synced open link in pullRequests[] bypasses inactivity settlement even when Keep threads with a PR is off. The early return prevents the retention setting and inactivity threshold from being evaluated.

Change

Open persisted links now block merge/close settlement while allowing the existing inactivity rule to run. Visible persisted links participate in the retention check. Unknown snapshots still block settlement, and terminal-link selection and existing candidate guards keep their behavior.

Scope and approval

Fixes #19728, following the maintainer-confirmed direction. This restores the behavior established by #9610 and the opt-in retention setting from #14642. The change is confined to the shared server settlement policy and its regression coverage; no settings defaults, contracts, or client code change. Adjacent #18937 and #18232 address different settlement rules.

Verification

Verified on macOS arm64, Node 24.21.0, pnpm 11.10.0, against main 21bedfb175.

• With upstream production code unchanged, the new tests produced four expected assertion failures: persisted open-link inactivity, both mixed open/terminal cases, and the SQLite worker with retention off. Retention-on and conservative controls passed.
• All 127 tests passed across ThreadSettlementService.test.ts, ProjectionSettlement.test.ts, and runtimeLayer.test.ts, including after rebasing onto current main.
• Policy coverage checks retention off/on, recent activity, disabled inactivity, unknown snapshots, mixed open/merged and open/closed links, terminal lookup input, existing terminal rules, and dismissed links.
• The runtime regression uses the real SQLite projection and orchestrator link/sync commands, reads the persisted settlement candidates, starts the real settlement worker, waits for its initial read and drains it without sleeps or polling, then verifies persisted settlement and shell state. With retention off it settles at the old activity timestamp; with retention on it stays active. Both preserve the links. Activity is seeded as an old user message projection; no provider session is launched.
• Server package typecheck, scoped lint/formatting, and git diff --check passed. Typecheck emits existing nonblocking Effect suggestions.
• Independent Codex review found no blockers and independently passed 14 focused policy/runtime cases.

Commands:

pnpm exec vp test run apps/server/src/orchestration-v2/ThreadSettlementService.test.ts apps/server/src/orchestration-v2/ProjectionSettlement.test.ts apps/server/src/orchestration-v2/runtimeLayer.test.ts
pnpm exec vp run --filter t3 typecheck
pnpm exec vp lint --report-unused-disable-directives apps/server/src/orchestration-v2/ThreadSettlementService.ts apps/server/src/orchestration-v2/ThreadSettlementService.test.ts apps/server/src/orchestration-v2/runtimeLayer.test.ts
pnpm exec vp fmt --check apps/server/src/orchestration-v2/ThreadSettlementService.ts apps/server/src/orchestration-v2/ThreadSettlementService.test.ts apps/server/src/orchestration-v2/runtimeLayer.test.ts
git diff --check

No live multi-day sweep, provider, UI, mobile, or remote-connection pass was run. Tests ran in a new isolated clone/worktree with task-owned resources; the user's live app and sessions were not used.

Implemented and independently reviewed with GPT-6 through Codex.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-16T11:29:21.864191Z	9543e95	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[macroscopeapp]

Approvability

Verdict: Approved at 612c5f4

Macroscope's review found this PR approvable — This is a narrowly scoped server bug fix that restores inactivity settlement for persisted open PR links without changing settings defaults or introducing new workflows. Comprehensive policy and runtime regression tests cover retention, mixed link states, unknown snapshots, and settlement-worker behavior.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

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: 95d6e1c8-ab9c-4136-a07d-e203226fd6e6

📥 Commits

Reviewing files that changed from the base of the PR and between 612c5f4 and 9543e95.

📒 Files selected for processing: 3

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

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

The settlement policy now permits inactivity settlement for threads with an open persisted pull-request link when pull-request retention is disabled. Unknown snapshots still prevent settlement. Unit and lifecycle tests cover link states, retention settings, and shell projection data.

Changes: Persisted pull-request settlement

Layer / File(s)	Summary
Settlement policy and unit tests apps/server/src/orchestration-v2/ThreadSettlementService.ts, apps/server/src/orchestration-v2/ThreadSettlementService.test.ts	Open snapshots no longer prevent inactivity settlement by themselves. Visible links count toward pull-request retention, and unknown snapshots still defer settlement. Unit tests cover open, terminal, and unknown snapshots with the relevant settings.
Runtime lifecycle coverage apps/server/src/orchestration-v2/runtimeLayer.test.ts	The lifecycle test checks settlement with retention disabled and no settlement with retention enabled. It verifies that shell settlement data and pull-request links match the persisted projection.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 9543e

The change appears mergeable after normal checks; no actionable settlement risk is identified.

Security architecture risk: 🔵 Low · up to 9543e

Idle threads with open pull requests can now settle when retention is disabled. Existing activity guards, per-project settings, and concurrent-change checks remain in place. No introduced security vulnerability was established, but the change also makes existing cleanup and configured settle scripts reachable for these threads.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The server-wide sweep can affect eligible existing threads across projects, subject to settings resolved for each project. Cleanup is addressed by the selected thread ID, and script execution uses that thread's project ID and worktree rather than a pull-request-supplied command or path.

Security Findings and Attack Paths

• inferred — No introduced security attack path was established in the inspected flow. Pull-request state affects eligibility, but an open link alone cannot trigger settlement without the inactivity rule and existing candidate guards. The change does not interpolate pull-request metadata into the configured script command.

Trust Boundaries and Controls

• observed — The unchanged projection query excludes deleted, archived, overridden, pinned, disabled, actively running, and pending-request threads. Policy checks additionally guard background work and queued turn starts. Idle-terminal closure inspects subprocesses and activity changes before closing a session.

Resilience and Maintainability Implications

• inferred — This service's cleanup is not a durable recovery queue: it reacts to live settlement events, while startup candidate selection excludes already-settled threads. Interruption can therefore leave persisted settlement without this service replaying missed cleanup. This mechanism predates the PR; the inspected evidence does not establish a new security consequence from it.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main server fix: inactive threads with persisted open pull-request links can settle.
Description check		The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the behavior, linked issue, implementation scope, focused test results, commands, and limi…
Linked Issues check		The changes satisfy the coding requirements in #19728. resolveAutoSettlementAt now blocks unknown snapshots, clears open-link candidates, and continues to retention and inactivity checks. The retent…
Out of Scope Changes check		The changes remain within #19728. Production code changes only the shared server settlement policy. Added tests provide direct regression coverage for the requested policy and runtime behavior. No set…

Full details: Description check

Explanation

The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the behavior, linked issue, implementation scope, focused test results, commands, and limitations.

Full details: Linked Issues check

Explanation

The changes satisfy the coding requirements in #19728. resolveAutoSettlementAt now blocks unknown snapshots, clears open-link candidates, and continues to retention and inactivity checks. The retention check includes visible persisted links. Unit tests cover retention off/on, inactivity disabled, mixed open and terminal links, unknown snapshots, terminal rules, and dismissed links. The runtime test verifies the persisted SQLite path and link preservation for both retention settings.

Full details: Out of Scope Changes check

Explanation

The changes remain within #19728. Production code changes only the shared server settlement policy. Added tests provide direct regression coverage for the requested policy and runtime behavior. No settings defaults, contracts, client code, or unrelated settlement rules change.

✨ 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.}

[juliusmarminge]

Taking over this one. I’ll reduce the tests to the useful coverage, update the implementation against current main, and verify it before publishing the changes.