feat(web): pull request surfaces — filters & qualifiers, all-server listing, update branch, reactions, in-place editing, smarter diffs - #6039
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.
🔍 Agent activity — reasoning, no tool calls
💭 Looking at the diff to understand what changed in the server files, particularly focusing on how the Effect service conventions are being applied. I'm examining
GitHubReactionSubjectErrorin GitHubPullRequestCli.ts, which usesSchema.TaggedErrorClasswith structured attributes likecommandandcwd, and has adetailgetter that produces the error message — this seems to follow the pattern for defining service failures with tagged error classes and organized attributes. However, I'm noticing the error doesn't capture the repository, number, or subjectId at the wrapping site, which feels like missing context that… The error class follows the existing pattern consistently. Looking at the GitLab side, I see decoders likedecodeOwnAwardIdJsonrather than services. ThesetReactionimplementation in the GitLab CLI usesEffect.gento yield aGitLabMergeRequestReadErrorwhen a decode fails, specifying "setReaction" as the operation. I'm noticing thatGitLabMergeRequestReadErroris being… I'm checking that the error construction at the failure boundary matches the file's style, and verifying Azure has theunsupportedhelper already in place for cases likesetReaction. Looking at the conventions now —Layer.mockin tests is fine as a test seam, andrepositoryIdentityOfbeing exported fromPullRequestService.tsworks as a pure helper function.… Now I'm checking the service conventions in the server code, particularly the new and modified service files. The pull request checks module is a pure utility with no service layer, which is fine. In the PullRequestService, I see thesetReactionmethod is usingPullRequestOperationErrorwith adetailfield, which follows the existing pattern, though the convention guide has some notes about this.… Now I'm checking the details around the GitHub CLI's service-instance passing and verifying thePullRequestService.setReactionnaming and gates look correct. The newpullRequestChecks.tsfile and any decoders appear fine, so I'm moving on to examine the web app's pull request state management in full, which usesAtom.familyand the app atom registry reference. Looking at the GitLab implementation,setReactionusesEffect.genwith proper error handling for decode failures. The GitHub side hasGitHubReactionSubjectErrorthat lacks repository/number/subjectId context, but checking the house style — sibling errors in the file likeGitHubViewerLoginUnavailableErrorandGitHubDiffCursorErroralso carry minimal context, whileGitHubDiffRevisionsUnavailableErrorincludes a number. Since the convention permits both approaches and this mirrors the immediate neighbors, I'm not flagging it as an issue. Now checking the comment placement in the diff — the new helper functionspullRequestNodeIdandreactionSubjectBelongsToPullRequestare inserted before the`gh` resolves a bare `owner/repo`...comment, which means the comment still precedesrepositoryArgsas expected. That earlier finding was addressed, so no need to repeat it. The helpers referencegraphqlReadwhich... SincegraphqlReadandparseRepositorySelectorare defined later inmakeas const arrow functions, calling them inside the helper bodies works fine due to lazy evaluation — not a convention issue. Looking at the other files likePullRequestReactions.tsxandPullRequestService.ts, they don't seem to violate any conventions either. Let me checkGitHubPullRequestProvider.tsfor any actual violations. Theunsupportedhelper in the… Checking the GitHubPullRequestClisetReactionimplementation against the helper error construction rule — it passes. The new code also follows the convention for acquiring Effect service dependencies from the environment, and the factory function takes pure values rather than service instances. The new interface additions toGitHubPullRequestCliare defined inline in Context.Service as expected. The data shapes likeGitHubPullRequestDiffSliceandGitHubBaseComparisonare properly separated from service shapes. The client state module inapps/web/src/state/pullRequests.tsusesAtom.makeappropriately at the app boundary, and thePullRequestProviderApiinterface in the server module follows the pre-existing port interface pattern rather than being a Context.Service. I'm verifying a few more spots to be thorough, particularly around theGitLabPullRequestCli.setReactionerror handling where there's a potential mismatch between the operation name and the error class name. The rule targets redundantoperation/reason/kind/phasefields that duplicate the error tag's distinction, butcommand: "gh"is a singleton field that's consistent with every other error class in the file—deviating from it would be inconsistent and arguably unnecessary cleanup. The new error class is properly added to the union and used correctly inGitHubPullRequestCli.ts, so… I'm checking the imports across the codebase—the pull request list logic file has the right Schema import, and in the GitHub CLI file I'm updating the import statement to include the named functionresolvePullRequestAuthorFilteralongside the type imports from the contracts package, which aligns with the convention for whole package imports.…