fix(server): settle inactive threads without a PR lookup - #10103
Effect Service Conventions: no issues found
All clear
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/effect-service-conventions.md
More information about how Check Run Agents work can be found in our Help Center.
Reviewed apps/server/src/orchestration/ThreadSettlementReactor.ts (the only in-scope changed file; ThreadSettlementReactor.test.ts is excluded by the check's patterns).
Checked:
- Imports: all Effect modules already imported as namespaces from subpaths; local service modules (
GitManager,PullRequestService,ServerSettings,OrchestrationEngine,ProjectionSnapshotQuery) imported as namespaces; named imports limited to pure helpers/types (ThreadSettlementPolicy,@t3tools/contracts). No changes here. - Service definition: unchanged
Context.Servicetag with inline interface,make, thenlayer; newsettleThreadis a localEffect.fninsidesweep, not a new service or shape interface. - Dependency acquisition:
settleThreadcloses over services obtained withyield* Foo.Fooinmake; no service instances passed as parameters (pullRequest: SettlementPullRequest | nullis an immutable domain value), no module globals, noManagedRuntime/runPromiseintroduced. - Errors: no new error classes, factories, predicates, or catch operators added; the relocated
Effect.catchCause+Cause.hasInterruptsOnlyhandling is the file's pre-existing pattern moved verbatim. - Change discipline: no new lint/type-checker suppressions; the behavior change (settling inactive threads before host-dependent lookups) is covered by two new focused tests that stub only external lookups via the existing harness layers.
🔍 Agent activity — 1 tool call
1. browse_code — apps/server/src/orchestration/ThreadSettlementReactor.ts
💭 I notice
Map.groupByis a valid JS builtin, so that's fine as pre-existing code. Checking the changed lines for convention issues—imports look unchanged, no new errors introduced, though the test's use of adetailfield onPullRequestOperationErrorcatches my eye since that's outside the scope but worth flagging as a possible convention violation. Looking closer atsettleThreadandlookupCandidates, theEffect.catchCauseusage seems like a pre-existing pattern rather than something newly introduced, and the concurrency settings and thread-or-null return look reasonable. TheThreadSettlementPolicyimports are unchanged pure helpers, so nothing stands out there either—overall this looks clean with no real findings to report. I do noticeCause.pretty(cause)being logged in the moved code, which technically touches on the "no arbitrary defect text in log annotations" guidance, but since this is a relocation of an existing repo-wide pattern rather than something newly introduced, flagging it would demand a broader cleanup beyond scope. I'll leave that unflagged and finalize with "All clear" as the summary.
Agent Credits: 22 credits
Token usage: 4 uncached input · 18,450 cache-read input · 23,535 cache-write input · 1,830 output
Agent Credits may also include non-token charges from external tools such as web research.