Skip to content

fix(server): refresh only threads for the merged PR - #10114

Closed
t3dotgg wants to merge 5 commits into
t3code/debt-gzuf-3-server-pr-statefrom
t3code/debt-gzuf-3-server-pr-association
Closed

t3dotgg wants to merge 5 commits into
t3code/debt-gzuf-3-server-pr-statefrom
t3code/debt-gzuf-3-server-pr-association

fix(server): retire PR lookup caches with merge confirmations

a537f84
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - Effect Service Conventions succeeded Sep 6, 2026 in 2m 44s

Effect Service Conventions: All clear

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 changed non-test TypeScript in scope: apps/server/src/git/GitManager.ts, apps/server/src/orchestration/ThreadSettlementReactor.ts, apps/server/src/pullRequest/PullRequestService.ts.

Checked: namespace subpath imports of Effect modules (unchanged and correct), the new observePullRequestMerge member declared inline on the existing Context.Service interface, dependency acquisition via yield* Foo.Foo inside make (no injected service parameters, no ManagedRuntime/runPromise in service code), error construction (GitManagerError/PullRequestOperationError used at failure boundaries with existing structured attributes; no new cause-derived messages), and change discipline (no new lint/type/diagnostic suppressions in non-test files; focused tests added for the new merge-observation behavior in GitManager, ThreadSettlementReactor and PullRequestService).

Suppression directives (@effect-diagnostics-next-line preferSchemaOverJson:off) appear only in *.test.ts files, which are excluded from this check's scope, and each carries an adjacent explanatory comment.


🔍 Agent activity — 8 tool calls

💭 I'm reviewing the changed lines in GitManager.ts, ThreadSettlementReactor.ts, and PullRequestService.ts for convention violations, noting that the mutable state in GitManager's make closure is scoped per-instance within Effect.gen, so it's fine. I'm considering whether JSON.stringify usage in prIdentity triggers the preferSchemaOverJson diagnostic rule, but that concern mainly applies to test files where suppression comments are added, which are out of scope here since only non-test file changes matter. I should just look at the actual diffs to check the error handling and detail message patterns are consistent with existing conventions.

1. browse_code — apps/server/src/git/GitManager.ts

2. browse_code — apps/server/src/git/GitManager.ts

💭 Let me check the observePullRequestMerge implementation and look at the branchPullRequest region.

3. browse_code — apps/server/src/git/GitManager.ts

💭 I'm checking whether using DateTime.makeUnsafe here fits the conventions, since throwing on invalid input inside an Effect that's typed as never for errors would just become a defect—doesn't quite match a listed convention though. Moving on to look at ThreadSettlementReactor.ts next. Now examining PullRequestService: runAction returns a SupportedProject rather than a string, and there's a readSummary refactor to check. PullRequestMergeEvent gets extended with provider/url/repositoryKey fields, which looks fine as a plain interface, and I want to verify the SourceControlProviderKind import was already there before moving to ThreadSettlementReactor.ts.

4. browse_code — apps/server/src/orchestration/ThreadSettlementReactor.ts

💭 Conventions check out, and the use of the global Map.groupBy is pre-existing so that's fine. Now moving on to look at the changed lines in PullRequestService.ts.

5. browse_code — apps/server/src/pullRequest/PullRequestService.ts

6. browse_code — apps/server/src/pullRequest/PullRequestService.ts

💭 The toPullRequestError mapper helper looks pre-existing and fine. Now I'm checking the changed lines around 2500 in PullRequestService for the merge event publish logic.

7. browse_code — apps/server/src/pullRequest/PullRequestService.ts

8. browse_code — apps/server/src/pullRequest/PullRequestService.ts

💭 I'm checking whether the logWarning call that logs the full error object is actually part of this PR's diff or just pre-existing code, since logging unbounded error objects could violate the safety convention around not exposing raw command output. I'm scanning GitManager.ts for convention issues too — JSON.stringify usage seems pre-existing elsewhere so it's not flagged, and the DateTime.makeUnsafe call on externally-supplied merge timestamps could throw a defect but doesn't clearly map to a listed convention, so I'll likely skip it. Checking the closures holding merge revision state — that matches existing caching patterns in the codebase, so it's not a dependency-hiding violation. I also confirm no new lint suppressions were added outside test files, tests were added for the behavior change, and the type imports and interface additions all look consistent with conventions. No findings here. The merge event object passed to observePullRequestMerge includes extra fields beyond what's needed, but that's structurally fine. I'll finalize with "All clear" as both the title and summary for the completion call.


Agent Credits: 64 credits

Token usage: 12 uncached input · 242,845 cache-read input · 61,437 cache-write input · 4,023 output
Agent Credits may also include non-token charges from external tools such as web research.