Skip to content

feat(orchestration): persist run-next messages on the server

MacroscopeApp / Macroscope - Effect Service Conventions failed Aug 16, 2026 in 4m 26s

Effect Service Conventions: 2 issues found

The new Effect service added by this PR does not follow the canonical service-module conventions.

  • apps/server/src/persistence/Services/ProjectionQueuedTurns.ts — retains a standalone ProjectionQueuedTurnRepositoryShape interface instead of defining the service interface inline in the Context.Service declaration, and splits the tag from its construction/layer instead of living in one canonical module (apps/server/src/persistence/ProjectionQueuedTurns.ts).
  • apps/server/src/persistence/Layers/ProjectionQueuedTurns.ts — owns construction but does not export make, exports the layer as ProjectionQueuedTurnRepositoryLive instead of layer, and annotates members via ProjectionQueuedTurnRepositoryShape[...] rather than ProjectionQueuedTurnRepository["Service"][...].

Both findings were posted as inline review comments.

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.

Methodology: reviewed every changed TypeScript file in scope plus the directly affected call sites (ProviderCommandReactor.ts, ProjectionPipeline.ts, ProjectionThreadMessages service/layer, contracts, client-runtime, web/mobile UI).

Checks that passed:

  • Effect library modules are imported as subpath namespaces; no consolidated named imports from "effect". import { AsyncResult } from "effect/unstable/reactivity" matches established repo-wide usage.
  • ProviderCommandReactor and the projection pipeline acquire ProjectionQueuedTurnRepository from the environment with yield* ProjectionQueuedTurnRepository; no service instances are passed as constructor parameters in production code.
  • No new ManagedRuntime.make, runPromise, or runPromiseExit in domain/persistence code; Effect.runPromise appears only in tests.
  • No new error classes, so no Schema.TaggedErrorClass / discriminator / cause issues. Effect.catchCause with Cause.hasInterruptsOnly for the queued-turn recovery is a structural predicate, which is acceptable.
  • Backend behavior changes are covered by focused tests (decider.queuedTurns.test.ts, ProviderCommandReactor.test.ts, migration 041 test, threadReducer.test.ts).

Context for the findings: the repository already contains canonical single-file services (persistence/AuthSessions.ts, persistence/AuthPairingLinks.ts, persistence/ProviderSessionRuntime.ts, the latter leaving only a @deprecated alias behind in Layers/). The legacy Services/ + Layers/ split for other projection repositories is pre-existing code and was not flagged.


🔍 Agent activity — 1 tool call

💭 Looking at the diff, I'm noticing some Effect service convention violations in the new projection queue turns file. The code is defining a separate ProjectionQueuedTurnRepositoryShape interface instead of keeping it inline with the Context.Service declaration, which goes against the established pattern. Additionally, the file structure should be consolidated into a single persistence/ProjectionQueuedTurns.ts file rather than split across Services/ and Layers/ directories. I'm also considering whether this new service should follow the existing repository pattern of splitting Services and Layers, or if the authoritative conventions should take precedence. Since this is a newly created service and the instructions are clear about applying the canonical convention when creating new Effect services, the split pattern here appears to be a violation that should be addressed. Additionally, I'm noticing that ProviderCommandReactorLive uses Layer.provideMerge to inject the ProjectionQueuedTurnRepositoryLive dependency directly, which bakes in a specific implementation rather than exposing the requirement in the layer's type signature — that's another potential violation of the production service construction pattern. I'm noticing the persistence directory still maintains the Services/Layers split for projections, while some files like AuthSessions.ts and AuthPairingLinks.ts have already been migrated to the new pattern. Since the instructions say to apply conventions when creating or consuming an Effect service and treat new patterns as authoritative, I need to decide whether to flag this inconsistency in the new ProjectionQ service. The new service splits into separate files and uses a standalone interface instead of inlining it in Context.Service, which diverges from the instructions. While the surrounding sibling repositories all follow the split pattern, the guidelines are explicit about treating new conventions as authoritative. This seems like a legitimate finding to flag, though I should verify by examining the migrated examples and the new files more closely to confirm what the canonical pattern actually is. I'm looking at the established pattern in this repo where migrated modules like AuthSessions.ts are structured as single files containing the interface, make function, and layer export all together. Let me check what the layer export is actually named in AuthSessions.ts. I found the canonical pattern in the codebase—the new ProjectionQueuedTurns service follows the legacy split approach with a standalone ProjectionQueuedTurnRepositoryShape interface rather than defining it inline within Context.Service. The service implementation should be consolidated into a single file. Looking at the actual implementation, the Layers file is missing the exported make function, and it's exporting the layer as ProjectionQueuedTurnRepositoryLive instead of the standard layer name. There's also a type inconsistency where it's using ProjectionQueuedTurnRepositoryShape["upsert"] instead of the proper service type path. I'm also noticing that ProviderCommandReactor is directly baking in the ProjectionQueuedTurnRepositoryLive implementation through Layer.provideMerge, which violates the convention that production services should acquire their dependencies from the environment using yield* rather than hardcoding concrete implementations. The error handling and schema validation patterns look consistent with the rest of the codebase, so those aren't issues. The catchCause and Schema.is checks follow existing patterns in other layer files. I'm checking whether importing AsyncResult as a named import from the effect/unstable/reactivity subpath violates the convention, since it's not a consolidated import from the root "effect" package—I should verify how the rest of the codebase handles similar imports. The AsyncResult import is standard across the codebase, so that's not an issue. The real problem is that this new Effect service was created using the legacy split-file pattern with a separate Shape interface and ...Live layer naming, but the established convention—already followed in migrated modules like the auth and provider session services—requires everything in a single canonical file with an inline interface, an exported make function, and export const layer. The style guide explicitly mandates this approach, so I need to determine if this deviation warrants a failure. There's a counterargument: the instruction warns against demanding unrelated repository-wide cleanup, and this new service mirrors the pattern used in twelve sibling projection repositories that feed into the ProjectionPipeline. But the authoritative guidance is clear, so I'll flag this as one finding with two inline comments—one on the Services file for the split structure and Shape interface, and one on the Layers file for the incorrect layer naming and missing make export. Since the check defaults to failure when findings exist, I'll mark it as such. Before finalizing, though, I should verify whether the repository has documented conventions in something like AGENTS.md that explicitly require new persistence projections to follow the Services/Layers pattern. This looks good for flagging — the repo follows a single canonical module pattern, and the new service uses the legacy split approach. Now I'm checking if there are other convention issues in the diff, particularly around how ProviderCommandReactor uses Layer.provideMerge(ProjectionQueuedTurnRepositoryLive) to hide the requirement, and whether ProviderCommandReactorLive already does this with other repositories elsewhere in the codebase. The provideMerge pattern is consistent with how ProjectionPipeline already handles things, so that's fine. Both ProviderCommandReactorLive and ProjectionPipeline provide their own instances of ProjectionQueuedTurnRepositoryLive, but since the repository layer is stateless and SQL-backed, they're accessing the same database anyway — no violation there. I'm also scanning for other potential issues: the schema definitions look correct, no new errors, no misuse of Layer.succeed, and runPromise only appears in tests. One thing to flag is that the new Layers file references ProjectionQueuedTurnRepositoryShape["upsert"] directly, which ties back to the standalone Shape convention issue I already noted. I need to post two inline comments suggesting consolidation: first, the Shape interface in Services/ProjectionQueuedTurns.ts should be inlined into the Context.Service declaration with all schemas, tags, and layer definitions in one canonical module. Second, the Layers/ProjectionQueuedTurns.ts file is missing explicit make and layer exports that should be added to the canonical module. I'm determining the exact line ranges for these comments now — the Services file is 69 lines total, so I'll target lines 46-69 for the Shape interface, and the Layers file needs a comment around the make definition and the final exports. I'm composing a review with two inline comments—one on the Services file covering lines 46-69, and another on the Layers file for lines 194-197 addressing the unexported make function and the naming convention issue with the Shape property accessor. I'll mark this as a failure state with the title "Effect Service Conventions: 2 issues found" to highlight the convention violation in the newly created service.

1. modify_pr — Reviewed the new Effect service introduced by this PR (ProjectionQueuedTurnRepository) and the touched service consumers. Two convention violations in the new service module layout; the rest of the changes (namespace imports, yield* Foo.Foo dependency acquisition in ProviderCommandReactor/ProjectionPipeline, no new runtimes or errors) look consistent with the conventions.


Agent Credits: 189 credits