fix(server): keep provider sessions alive during background agent work - #4199
ChamaruAmasara wants to merge 5 commits into
Effect Service Conventions: No issues found
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.
Reviewed the added touchLastSeen service operation across ProviderSessionRuntime (persistence repository), ProviderSessionDirectory (Services shape + Layers implementation), and ProviderService (runtime-activity throttle wiring), plus the updated test layers.
Findings:
- Effect modules are imported as subpath namespaces (
import * as Clock from "effect/Clock", etc.); no consolidated named imports from"effect"were introduced. - The new
touchLastSeenmethods are declared inline within the existingContext.Serviceinterface (ProviderSessionRuntimeRepository) and referenced viaService["Service"], matching the canonical pattern already used byupsert/getByThreadId. - Error handling reuses the established
toPersistenceSqlOrDecodeError/toPersistenceErrormappers with structured operation + correlation context; no new generic error wrappers orcause.message-derived messages were introduced. Effect.catch(() => Effect.void)inrefreshLastSeenForActivityintentionally handles the entire error channel for a documented best-effort touch, which is an allowed use ofEffect.catch.- No
ManagedRuntime.make,runPromise,Layer.succeedsmuggling of runtime-backed services, or fake dependency injection was introduced; dependencies remain acquired from the Effect environment.
The standalone ProviderSessionDirectoryShape interface predates this PR (the change only appends one method); flagging it would constitute unrelated repository-wide cleanup, which is out of scope. No convention violations were introduced or retained in the changed scope.
🔍 Agent activity — reasoning, no tool calls
💭 I'm examining the diff to check for violations against Effect service conventions, starting with the key files like ProviderSessionRuntime, ProviderService, and ProviderSessionDirectory across both the Services and Layers directories. I need to correct the URL format I'm using. I need to reconsider how I'm structuring this parameter to get it right. I'm noticing the URL parameter has an unwanted leading newline, so I'll try reformatting it to keep everything on a single line instead. The branch reference isn't working, so I'll switch to using the head commit SHA instead. I'm noticing that
ProviderSessionDirectoryuses a standaloneProviderSessionDirectoryShapeinterface instead of defining it inline within the service declaration, which goes against the established conventions that say to avoid retaining separateFooShapeinterfaces. Since this is existing code, I need to consider whether to refactor it to align with the standards or leave it as-is. The key insight is that the PR only adds atouchLastSeenmethod to the existing shape—it doesn't introduce the standalone interface pattern itself. The conventions clarify that I should only flag violations introduced or retained within the PR's changed scope, and I shouldn't demand unrelated repository-wide cleanup. Since the shape predates this PR, flagging it would be asking for cleanup outside the PR's scope, so I'll skip this and focus on actual violations in the changed code. The imports are set up correctly with Effect modules as namespaces and Clock imported from the right place. Looking at howrefreshLastSeenForActivityusesEffect.catchto swallow errors withEffect.void— that aligns with the convention since the entire error channel is being intentionally handled as a best-effort operation. But I'm noticing the convention statement seems to continue beyond what I'm seeing here. Now I need to examine the ProviderSessionRuntime.ts changes, particularly thetouchLastSeenimplementation and how errors are being mapped. I should also check whether ProviderSessionRuntimeRepository is using a standalone shape or inline interface definition. The inline interface pattern in the Context.Service looks good and aligns with the existing conventions. The touchLastSeen changes fit naturally with the established patterns. Now I need to examine the error handling logic intoPersistenceSqlOrDecodeErrorand how it's used in the touchLastSeen implementation, particularly around the relevant code sections. Checking whether thetouchLastSeenmethod follows the same patterns asupsertand whether the ProviderSessionDirectory implementation maintains consistency with existing conventions. The error handling looks correct, and I'm verifying that the imports use proper namespacing and the service definitions don't introduce any violations. ThetoPersistenceErrorhelper anddetailfield follow established patterns in the codebase, and the newtouchLastSeenfunction reuses this helper consistently. Thedetailfield here uses a static string rather than copying fromcause.message, which aligns with existing conventions. Everything mirrors the established patterns with properly namespaced imports. TheProviderSessionDirectoryservice uses a standaloneProviderSessionDirectoryShapeinterface, but since this PR only adds a method to it rather than creating it, this falls outside the scope of unrelated cleanup. I don't see any convention violations introduced or retained within the changed scope—the changes are clean and follow established patterns. The check defaults to failure when findings exist, but with no violations detected, this should resolve to success.