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: 5506d59e-e0ea-4d7a-8bc5-9877a42b76f4

📥 Commits

Reviewing files that changed from the base of the PR and between 9543e95 and 7cce90c.

📒 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; 1 remain after this review.

📝 Walkthrough

The settlement policy now permits inactivity settlement for threads with open persisted pull-request links when retention is disabled. Unknown snapshots still prevent settlement. Unit and lifecycle tests cover link states, retention settings, and persisted shell 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. Persisted links count toward pull-request retention. Unknown snapshots still defer settlement. Unit tests cover open, terminal, and unknown snapshots with the related settings.
Runtime lifecycle coverage apps/server/src/orchestration-v2/runtimeLayer.test.ts	The lifecycle test checks settlement with retention disabled and retention enabled. It verifies persisted links avoid branch and summary lookups and that pull-request shell data remains unchanged.

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 7cce9

Idle threads with an open pull request now settle after the inactivity threshold when "Keep threads with a PR" is off, and stay active when it is on. Threads whose pull-request status is unknown are still never settled automatically. The policy change is small and covered by focused unit and lifecycle tests. No merge-blocking risk was identified.

Security architecture risk: 🔵 Low · up to 7cce9

The change allows inactive threads with open pull requests to settle when retention is disabled. Existing retention and activity safeguards remain. No security defect was identified, but the added tests do not directly exercise interruption or recovery.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed behavior can affect otherwise eligible inactive threads with persisted open links wherever retention is disabled. Cleanup remains addressed to the selected thread; configured settle scripts run only when that thread has an existing dedicated worktree, not in the shared checkout.

Trust Boundaries and Controls

• observed — In the inspected cleanup path, pull-request metadata influences settlement eligibility but does not supply a shell command. Script selection comes from resolved project settings; execution receives the selected thread ID, project ID and worktree path. No new script authority or metadata-to-command flow was identified.

Resilience and Maintainability Implications

• observed — Existing cleanup controls limit interference with resumed work: settlement is rechecked after I/O, script starts are deduplicated by settlement timestamp within the service instance, and idle-terminal closure uses a thread lock plus fresh subprocess and activity checks. These controls predate the PR and remain applicable to the newly eligible path.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: allowing inactive threads with persisted open pull-request links to settle.
Description check		The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains expected behavior, links the issue and maintainer approval, describes focused tests and …
Linked Issues check		The implementation satisfies #19728. resolveAutoSettlementAt blocks links with unknown snapshots. It clears the merge/close input when a visible persisted link is open, then evaluates candidate guar…
Out of Scope Changes check		The changes remain within #19728. Production changes are limited to the shared server settlement policy. Unit and runtime tests provide regression coverage for the policy and persisted projection path…

Full details: Description check

Explanation

The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains expected behavior, links the issue and maintainer approval, describes focused tests and observed results, and states checks that were not run.

Full details: Linked Issues check

Explanation

The implementation satisfies #19728. resolveAutoSettlementAt blocks links with unknown snapshots. It clears the merge/close input when a visible persisted link is open, then evaluates candidate guards, retention, and inactivity. Visible persisted links count for autoSettleKeepPullRequests. Tests cover retention off/on, recent activity, disabled inactivity, unknown snapshots, mixed open and terminal links, terminal-link rules, and dismissed links. The reported runtime test covers the persisted SQLite settlement path and preserves links.

Full details: Out of Scope Changes check

Explanation

The changes remain within #19728. Production changes are limited to the shared server settlement policy. Unit and runtime tests provide regression coverage for the policy and persisted projection path. The supplied change summary reports no settings default, contract, client, or unrelated settlement-rule changes.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 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.