Skip to content

fix(orchestration-v2): harden reused subagent recovery

MacroscopeApp / Macroscope - Effect Service Conventions failed Jul 29, 2026 in 3m 1s

Effect Service Conventions: 1 issue found

Found one Effect service convention violation in the changed scope.

  • apps/server/src/orchestration-v2/ProviderTurnStartService.ts (lines 436-438): a known tagged failure (ProjectionStoreThreadNotFoundError) is recovered with Effect.catchIf(Schema.is(...)). The conventions require statically known tagged failures to be caught with Effect.catchTags({ ... }), even for a single tag; catchIf is reserved for genuinely structural predicates. This is also the only catchIf(Schema.is(...)) occurrence in the repo, whereas Effect.catchTags is used pervasively elsewhere.

An inline review comment with a suggested fix has been posted.

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.

Scope reviewed: changed TypeScript under the configured include patterns, focusing on new/refactored Effect service code.

Checked and cleared:

  • import { layer as ... } aliases in the new ProviderTurnStartService.test.ts and CodexAdapterV2.test.ts — this alias style is an established, repository-wide pattern (used throughout runtimeLayer.ts, applicationLayer.ts, harnesses, and existing tests), so it is out of scope for this PR and not flagged.
  • Layer.mock(...) usage in ProviderTurnStartService.test.ts — legitimate test seams; passing mocked service instances in tests is permitted.
  • New type ProviderAdapterV2ExistingSubagent in ProviderAdapter.ts — pure data interface, no service/error concerns.
  • RunExecutionService.ts / CodexAdapterV2.ts changes — routing/logic and pure helpers; no new service definitions, error classes, module-global runtimes, or ManagedRuntime/runPromise boundaries introduced.

No violations found related to service definition ordering, error modeling, dependency acquisition, or runtime boundaries.


🔍 Agent activity — 1 tool call

💭 I'm examining the diff to check for Effect service convention violations, starting with the new test file and its imports from the Effect library. Looking at the imports in the test file, I notice several that use aliases to erase the module namespace—specifically import { layer as idAllocatorLayer } and import { layer as providerTurnStartLayer, ProviderTurnStartServiceV2 }. The convention flags these kinds of aliases as violations since they hide the original module namespace. I need to verify whether this is an existing pattern in the codebase or a new violation being introduced. I need to focus on the actual service convention violations rather than getting caught up in stylistic details. Let me examine the production code changes to see if there are real issues with how services are being defined or consumed, then check whether the test file's import patterns match what's already established in the codebase. I'm noticing that ProviderTurnStartServiceV2Shape is pre-existing code that wasn't modified in this PR, so it's outside the scope of what I'm reviewing. Now I'm looking at the catchIf usage that was added in ProviderTurnStartService.ts, where it's filtering projection store results based on a schema check. The convention is clear here: when catching statically known tagged errors like ProjectionStoreThreadNotFoundError, I should use Effect.catchTags with an object mapping the tag to its handler, rather than catchIf with a schema predicate. This is a violation in the new code that needs to be fixed. I'm noticing that ProjectionStoreThreadNotFoundError is a tagged error class, and the current implementation uses Effect.catchIf with a schema check, but the convention would be to use Effect.catchTag instead to match the error handling pattern. Actually, looking more carefully, the convention is to use Effect.catchTags with an object mapping the error tag to its handler, so it should be Effect.catchTags({ ProjectionStoreThreadNotFoundError: () => Effect.succeed(null) }). This is a violation in the changed scope since it's a new line added in this PR within a file matching the include pattern. Let me check the exact line number in the file to confirm. I'm checking whether catchIf with a schema predicate appears elsewhere in the codebase, though the convention should take precedence regardless of existing patterns. Let me reconsider this approach. The convention explicitly prohibits using catchIf with schema predicates to recover known _tag variants, which is exactly what's happening here—so this needs to be flagged. Moving on to the other changed files, the test file uses Layer.mock which aligns with the convention allowing service instances in tests. However, the imports like import { layer as providerTurnStartLayer } and import { layer as idAllocatorLayer } are erasing the module namespace at a service boundary, which the convention says to flag even in mechanically updated tests. But I should check whether this pattern is already established in the codebase—the existing test file CodexAdapterV2.test.ts already uses layer as idAllocatorLayer, so this might be a pre-existing convention in the repo. The strongest, most defensible finding remains the catchIf with schema predicate violation. Let me scan the changed files systematically for other instances of catchIf, catchTag, Layer.succeed, or similar error-handling patterns that might violate the conventions. I need to check how prevalent the layer as aliasing pattern is across the codebase to determine if this is truly a violation or an established convention in this repo. The layer as X import alias pattern is standard throughout the codebase in both production and test files, so that's not worth flagging as an issue in this PR. Now I need to check whether the repo uses the catchIf pattern with schema predicates elsewhere. I'm noticing this is a new violation introduced in the PR—it's using catchIf with a schema predicate when it should be using Effect.catchTags instead, which is the established convention. Since this is the only instance, it's worth flagging, and I should check how other parts of the codebase handle similar error recovery patterns. I'm noticing that Effect.catchTags is the standard pattern used throughout the codebase, so the new catchIf(Schema.is(...)) approach in ProviderTurnStartService.ts stands out as a deviation from both the established convention and the linting rule. This is a valid concern. Let me scan through the diff again to check for any other service definition, error handling, or dependency issues I might have missed, starting with the main production changes around CodexAdapterV. Preparing the inline suggestion with proper indentation to replace the catchIf block with catchTags, targeting lines 436-438.

1. modify_pr — Effect service conventions: one issue found. Known tagged failures should be recovered with Effect.catchTags rather than catchIf + a schema predicate.

💭 Now I'm generating the JSON output for the check run, marking it as a failure since a convention violation was detected.


Agent Credits: 67 credits