feat(grok): native ACP parity (catalog, effort, usage, plan, multi-agent, compact) - #5503
EnzoTironi wants to merge 7 commits into
Effect Service Conventions: 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 for Effect service conventions, I'm checking how imports are consolidated and whether error handling in GrokAdapter is intentional—specifically whether
Effect.catchis properly handling the error channel whenoptions.onAvailableCommandsreturns an Effect with no error type. The rest of the changes look fine:Effect.catch(() => Effect.void)in sendTurn is appropriate, no new error classes are introduced, and the service definition changes like the getter forstreamChangesdon't violate conventions. Now I'm examining the imports more carefully across the changed files—GrokAcpSupport.ts, GrokProvider.ts, and GrokAdapter.ts all use named imports from contracts and shared packages, which aligns with the conventions. Looking at this error message in the code, I'm checking whether it violates the convention about not copyingcause.messageinto a detail field. The diff shows the variable was renamed but the mapError with the detail field was already there before this change, so it's not being newly introduced—just retained in the modified scope. Now I'm examining GrokAdapter's error handling patterns, particularly looking at howEffect.catchis used and whether they're following the convention of catching statically known tagged failures withcatchTags. The catch handler that returnsEffect.voidfor the entire channel looks fine. I should also verify that no new service definitions were added and check the import consolidation across the touched files to ensure they're pulling from "effect" consistently. The codebase relies heavily on Effect.catch for error handling, and new code follows that established pattern. Looking at the new additions, there are no new error classes or service definitions to worry about, and the dependency setup withmakeGrokAdapterusing options and callbacks fits the existing conventions. The helper functions in GrokProvider likecapabilitiesFromGrokModelMetaare straightforward utilities, and AcpSessionRuntime appears to be a Context.Service module. I notice GrokAdapter.ts imports a pre-existing GrokAdapterShape from Services, which technically violates the convention of defining service interfaces inline, but that's legacy code outside the changed scope of this PR. Since this is a feature addition rather than a service refactor, I shouldn't demand unrelated cleanup. Checking the actual diff for Layer.succeed, ManagedRuntime, or runPromise calls in the changed code—none of those appear. Now looking at GrokAcpSupport.ts, theapplyGrokAcpConfigSelectionshelper accepts a runtime parameter typed as a service instance, which follows the existing pattern already used byapplyGrokAcpModelSelection. UsingFoo["Service"]to refer to the inferred interface is the correct convention, and accessing the service module via namespace import is fine here. Let me check the imports at the top of the file. I notice the GROK_PRESENTATION change removed the early access badge label, which is a behavioral change rather than a convention issue. Let me verify the AcpSessionRuntime.ts modifications follow the service definition conventions and check if any new error handling is copying cause.message to the detail field—though it looks like the GrokDriver's mapError already handles this pre-existing pattern. I'm checking the logging pattern in GrokAdapter where warnings are emitted with the cause included—that's acceptable. Now I need to verify there are no new imports from the "effect" library in the other modified files like the test utilities, CursorAdapter, or the text generation module, since those aren't in the provider directory scope. The imports look clean—there's just one pre-existing Effect import in an untouched file. Now I'm weighing the instruction to respond with "All clear" against the JSON schema requirement for structured output. The schema is a system-level constraint, so I should follow that and record the check conclusion in the proper format rather than a plain text response.
Agent Credits: 101 credits