Repository navigation
feat(web): drag pinned threads into your own order - #5581
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
💭 I'm checking the current diff for Effect service convention violations. The previous run caught a reused error class, but that's been fixed with a new
ThreadPinReorderUnsupportedError. Now I'm looking at the migration file to see if it's following the proper import conventions for the Effect library. The new error class in the hooks file looks good with the Schema.TaggedErrorClass pattern, and the orchestration decider is using the existing error pattern correctly. For the Sidebar.logic file, they're re-exporting a pure helper function from client-runtime with an alias rename — that's acceptable since it's a utility function, not a service layer, and the guidance allows named imports for pure helpers. Now I need to verify howCauseis being imported in the files that useCause.squashandCause.fail, and check whetherOptionis imported correctly across the changes. The prior issue is resolved. Now I'm checking the remaining violations—the mobileAlert.alertcall inuseThreadListActionsis just UI presentation, not a service convention problem. For the server-sideOrchestrationCommandInvariantError, thedetailfield contains structured information rather than just copyingcause.message, so that's acceptable. Themov Looking at themovePinnedThreadimplementation on mobile, it's usingAlert.alertto show an unsupported feature message inline rather than throwing a typed error like the web side does. This isn't really a violation of the error handling convention since the mobile approach is intentional — it's designed to show user-facing alerts for unsupported operations rather than propagating typed errors. I'm wondering if there's a consistent pattern here about when to use alerts versus typed errors. I'm noticing there's an orchestration-v2 subsystem handling thread pin/unpin operations that this PR doesn't modify, and the ServerEnvironment declares threadPinReorder as an active capability. If orchestration-v2 is the live command handler, dispatching thread.pin.reorder could fail there—that's a potential functional gap, though it's outside the scope of Effect service conventions. The check is specifically about service conventions: imports, service definitions, dependency acquisition, error handling, and file layout. A missing command case in orchestration-v2 would be a correctness issue rather than a convention violation. I should focus on what's actually in scope: whether this PR creates, moves, refactors, or consumes an Effect service. It's consuming client-runtime command atoms and server projections, so I need to examine the actual convention violations—import ordering, service definition patterns, and dependency acquisition in the affected files like Sidebar.logic.ts. Now I'm looking at the re-exports from client-runtime, particularly the alias rename ofsortPinnedThreadsByOrderKey. The service-boundary rules flag aliases that erase module namespaces, and there's a rule against leaving compatibility re-export shims in deleted service files. This re-export through Sidebar.logic is borderline—it's a pure helper shim, but consumers like useThreadActions.ts are importing through it instead of directly from client-runtime, which arguably violates the convention that consumers should import directly from the source module rather than through intermediate shims. The instructions say to mechanically update every consumer to the canonical path and not leave compatibility shims, though that guidance is framed around service/layer files. Since the check defaults to failure and I've already fixed one issue, I should look more carefully for genuine service-convention violations in the changed scope, starting with examining howEffect.fnand dispatch are used inpackages/client-runtime/src/operations/commands.ts. I'm working through the various schema and command patterns across the codebase — checkingthreadCommands.ts, the serverSchemas.tsre-export convention,projector.tsusage ofdecodeForEvent, the migration file, and theProjectionThreadsschema addition. The error handling in mobile'smovePinnedThreadwithCause.squash` and Mobile aligns with the existing local patterns, so that's settled. Now I'm wondering about the orchestration-v2 gap — whether that's actually the live path. The PR wires up decider.ts and ProjectionPipeline, with ServerEnvironment declaring the capability, but if Orchestrator v2 is the real command handler and it doesn't support "thread.pin.reorder", the feature would fail. The diff only added the command to orchestration.ts, not orchestrationV2.ts, so I should verify whether the v2 contracts need it too. The prior fix aligns with the codebase conventions—using namespace subpath imports for effects, the Schema.TaggedErrorClass pattern with structural attributes, and avoiding unnecessary Layer or service injection patterns. The only remaining question is whether the Sidebar.logic.ts re-export block fits the conventions for pure helper re-exports within the web UI layer.
Agent Credits: 88 credits