Skip to content

refactor(source-control): drivers build their own services and own ho…

MacroscopeApp / Macroscope - Effect Service Conventions completed Oct 24, 2026 in 15m 18s

Effect Service Conventions: 1 issue found

  • apps/server/src/sourceControl/SourceControlRepositoryService.ts:96: Provider error details, including upstream response text, are copied into a user-visible error without sanitization.

Details

Warning

  • Invalid effort level "max" in front matter. Fell back to "low". Valid values are: low, medium, high.

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 all 29 changed files (no prior completed review)

An earlier inline suggestion about sourceControlMediaResponse.ts was withdrawn in a PR comment; it is not a finding.


πŸ” Agent activity β€” 45 tool calls

πŸ’­ Inspecting likely violations I need to look into possible violations, particularly focusing on a log that contains a signed URL with an HTTP change. There's a preexisting log message that has been altered, noting the warning about the existing URL not being introduced earlier. I’m wondering if I should focus on backend behavior tests to see if this issue has any impact. I think my next step involves inspecting those tests further through some research.

1. browse_code β€” For PR head inspect changed Effect service code for convention violations, especially sourceControlMediaResponse asset route, builtInDrivers layer, withOwnServices, tests for new media behavior. Identify exact lines and whether focused tests exist.

2. browse_code β€” apps/server/src/assets/sourceControlMediaResponse.ts

3. browse_code β€” apps/server/src/sourceControl/builtInDrivers.ts

4. browse_code β€” apps/server/src/http.ts

5. modify_pr β€” Submitted review with 1 inline comment

6. browse_code β€” At PR head inspect focused tests for newly changed source control media response and media fetch behavior and whether test layers use mocked business logic, give test file paths and coverage succinctly.

7. browse_code β€” packages/source-control-core/src/server/driver.ts

πŸ’­ Evaluating comment accuracy I'm wondering if the comment about the rule regarding the "new exported free function" is accurate. It mentions something about the filesystem and Git processes regarding persisted states and fetching HTTP, potentially affecting server capabilities, which sounds a bit unclear. I think I'll need to analyze this further to ensure accuracy and clarity. Just making sure I understand all the implications before jumping to any conclusions!

8. browse_code β€” Check whether PR head adds any new lint/typechecker/static diagnostic suppression directives in changed TS production files, or named imports from bare effect, or catchTag usages in changed lines. Only list actual changed lines.

9. browse_code β€” Inspect packages/source-control-bitbucket/src/server/BitbucketSourceControlProvider.ts repositoryDetail helper and error detail flow. Does changed line introduce exposure of unbounded cause.message in user-visible detail? quote exact definitions.

10. browse_code β€” Find new focused tests at PR head for SourceControlRepositoryService changing provider detail propagation; specify test lines if any.

11. browse_code β€” Inspect SourceControlProviderError schema detail definition and SourceControlRepositoryService mapRepositoryError. Are provider errors passthrough structured domain errors part of target channel, or necessary mapping to repository error? report precise.

12. browse_code β€” Inspect actual tests for new sourceControlMediaResponse; do they use fake HTTP fetch or mock business logic? Justify if convention 'focused tests use test layers for external services only, never mocks of core business logic' is violated by changed production behavior.

13. browse_code β€” Inspect source-control-core server driver.ts new withOwnServices and builtInDrivers SourceControlDrivers tag: is Layer.build(services) used with Scope scope correctly, and do instances retain resources? Give concise assessment.

14. browse_code β€” Check source-control-github gitHubMediaFetch.make named Effect.fn and credential acquisition; any conventions failure due captured service closure? Distinguish service construction closures from module globals.

15. browse_code β€” Does sourceControlMediaResponse.ts new exported free function violate precise rule 'new exported free function that does a server capability's effectful work (filesystem, Git, processes, persisted state) and is called from a handler' even though its only effectful I/O is HTTP media fetch? assess relative to convention docs.

16. browse_code β€” Read docs/internals/effect-services.md placement section on HTTP media response helpers. Does generic transport policy function that delegates I/O to driver need Context.Service? brief.

πŸ’­ Clarifying comment issues I need to retract a comment, but it seems that the required node ID is unavailable. I can’t reply to the review since the comment ID isn’t there either. While I could post an issue comment to correct this, it might clutter things up. It looks like the existing comment might be server limitations regarding valid fetch capabilities. I think I should just keep the current state as is for now.

17. browse_code β€” Inspect PR changed sourceControlRepositoryService mapper and tests if detail is arbitrary provider string; is detail cause-derived? Is new passthrough a security convention breach?

18. browse_code β€” Find whether any built-in SourceControlProviderError.detail includes raw stderr, URLs query, credentials or cause.message, especially providers after mapper now passes all detail to UI. Give examples if present.

19. browse_code β€” apps/server/src/sourceControl/SourceControlRepositoryService.ts

20. modify_pr β€” Submitted review with 1 inline comment β€” not performed: the agent's request was refused before it ran

21. modify_pr β€” POST repos/{owner}/{repo}/pulls/{pull_number}/comments

22. browse_code β€” Assess whether previously posted inline note on apps/server/src/assets/sourceControlMediaResponse.ts line25 demanding service method is a clear Effect service convention violation or optional architectural preference. Answer yes/no.

23. modify_pr β€” Correction to my inline note on apps/server/src/assets/sourceControlMediaResponse.ts:25: please disregard the request to turn the response helper into a service method. It acquires the registered Effect service and delegates media fetching to the driver; the cited placement convention does not require a separate service for this transport-response helper.

24. browse_code β€” Does SourceControlRepositoryService.lookupRepository new provider detail passthrough apply to Forgejo case and user-facing RPC? Confirm exact path.

25. browse_code β€” Verify Forgejo getRepositoryCloneUrls calls requestFj or just cli? Does raw HTTP body path reach clone URL operation?

26. browse_code β€” Does Forgejo requestFj error response body read have size limit before creating error detail? inspect precise.

27. browse_code β€” apps/server/src/assets/*

28. browse_code β€” Are there other unsafe details used by changed mapper on clone/publish/lookup besides Forgejo; concise.

29. browse_code β€” Is sourceControlMediaResponse existing old GitHubMediaFetch free function analogous and therefore merely moved/refactored? Compare old diff in prompt if possible.

30. browse_code β€” Inspect sourceControlRepositoryService new mapping if cause.detail copied and detail earlier fixed Bitbucket only; is this a clear violation of rule safe bounded service error attributes?

31. browse_code β€” At PR head is GitHub provider clone URLs detail from upstream error message forwarded by repository mapper? identify lines.

32. browse_code β€” Does GitHub upstream message contain raw wire text? assess risk succinctly.

33. browse_code β€” No additional question. Summarize only strong convention finding.

34. browse_code β€” Is there a PR check finding requiring summary failure for unsafe error details? Answer yes/no.

35. browse_code β€” Is check configured to neutral conclusion? provide actual config heading lines.

36. browse_code β€” Read .macroscope/check-run-agents/effect-service-conventions.md frontmatter conclusion value only.

37. browse_code β€” Is 8MB error body safe bounded under Effect service conventions? no need.

38. browse_code β€” Does .macroscope check instructions recommend failure for clear violations or neutral by default?

39. browse_code β€” Check conclusion neutral config and findings. No more.

40. browse_code β€” What does error discipline say on detail copying cause.message?

41. browse_code β€” Is provider detail in fact cause.message with other name through provider mapper?

42. browse_code β€” Confirm exact conclusion config from .macroscope/check-run-agents/effect-service-conventions.md first 22 lines.

43. browse_code β€” No request.

44. browse_code β€” Summarize relevant result in 1 sentence.

45. browse_code β€” No further research.


Agent Credits: 268 credits

Token usage: 810 uncached input Β· 3,702,298 cache-read input Β· 603,918 cache-write input Β· 29,777 output
Agent Credits may also include non-token charges from external tools such as web research.