Skip to content

fix(orchestration): Preserve Claude subagent attribution after settle - #5388

Closed
mwolson wants to merge 222 commits into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/claude-subagent-postsettle-attribution
Closed

mwolson wants to merge 222 commits into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/claude-subagent-postsettle-attribution

fix(orchestration): Preserve Claude subagent attribution after settle

a45acc5
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - Effect Service Conventions succeeded Aug 11, 2026 in 2m 46s

Effect Service Conventions: No findings

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 check for Effect service convention violations across several files in the orchestration and client runtime modules. The changes involve adding a Semaphore import to the Claude adapter and modifying how provider continuation requests and thread workflows are handled. I'm checking whether the imports follow Effect v4 conventions — the test file uses effect/testing/TestClock which confirms v4 style, so effect/Semaphore should be valid. But I'm noticing the use of namespace type imports like import type * as Cause from "effect/Cause" in ProviderContinuationRequests, which might violate the convention against requiring namespace type imports when named imports would be more appropriate.… The convention… Looking at the PR changes, the main modifications are in an adapter factory that was already there — it's taking dependencies as parameters but that's not new. I'm checking whether there are any new service instances being introduced through the ClaudeAdapterV2 factory inputs, but those appear to be pre-existing. There's a new callback in the ProviderContinuationRequest interface called failIfCurrent that follows the same pattern as the existing dispatchIfCurrent and clearIfCurrent callbacks — it's a deliberate design choice for handling effects. The import ordering looks fine, and the client-runtime threadWorkflows change isn't Effect service related. Scanning through the ClaudeAdapterV2 diff, there are no new Layer.succeed or runPromise additions — just Effect.promise in tests. The return type annotation Effect.fn.Return<void> isn't a convention issue, and the string constants like CLAUDE_SUBAGENT_WAKE_DELIVERY_FAILURE aren't error types. For the logging, I'm seeing Effect.logWarning calls with bounded identifiers and a Cause being logged — need to verify whether logging a Cause directly violates the sanitization convention.… The test file has some pre-existing alias imports that technically violate the convention, but since they're not part of the diff—only the test bodies were added—they fall outside the changed scope and shouldn't be flagged. Though I'm uncertain whether "retained" violations count as something that needs reporting.… Looking at the import statement from the provider module, I'm checking whether the named imports violate the service boundary convention. Since the imports aren't using aliases that would hide the module namespace—the names are already qualified with ProviderContinuationRequests—this doesn't seem to be a violation of the pattern that flags namespace-erasing aliases. The added constant PROVIDER_CONTINUATION_MESSAGE_TEXT falls under the allowed category for named imports since it's a config value rather than a service member. The same reasoning applies to ProviderContinuationService.ts, which already imports from the same module without introducing any namespace erasure. Flagging this would be noisy since the existing import style predates this PR. Now I'm looking at a deeper architectural question: whether ClaudeAdapterV2's makeClaudeAdapterV2 function follows the convention for service construction. The function accepts dependencies like idAllocator, fileSystem, queryRunner, and continuationRequests as parameters, which is a pre-existing pattern for adapters in the codebase. rn: does the diff add anything like Layer.succeed calling runtime APIs? No. Any new ManagedRuntime? No. The PROVIDER_CONTINUATION_MESSAGE_TEXT constant placed before the interface in ProviderContinuationRequests.ts is fine — that's a valid ordering. The Context.Reference with a default value and separate layer effect is pre-existing, so no new violations there. I'm confident this is clean, so I'll report success with "All clear".…


Agent Credits: 230 credits