Skip to content

fix(server): skip undecodable provider runtime rows when listing sessions - #3951

Merged
juliusmarminge merged 5 commits into
mainfrom
fix/skip-undecodable-provider-runtime-rows
Jul 17, 2026
Merged

juliusmarminge merged 5 commits into
mainfrom
fix/skip-undecodable-provider-runtime-rows

refactor(server): collect decoded runtime rows once

3f61538
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - Effect Service Conventions succeeded Jul 17, 2026 in 2m 10s

>

The prior run flagged a raw payload leak from logging the full PersistenenceDecodeError in ProviderSessionRuntime.ts list. At the current head that concern is resolved: the log now records only the derived .message string (Decode error in {operation}: {issue}, where issue comes from summarizeSchemaIssue and contains only issue tag names) plus the threadId correlation id — both bounded and free of raw row data such as runtimePayload. No new Effect service convention violations were introduced. Imports use namespace subpaths (effect/Array, effect/Result, effect/Option), and Effect.catch legitimately handles the entire decode error channel to skip stale rows.

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 this PR change to ProviderSessionRuntime.ts, it's switching from failing on undecodable rows to skipping them instead. There was a previous concern about the PersistenceDecodeError logging leaking raw row payload, so I'm checking whether that issue got resolved in the current file state. I need to adjust the format for the repository URL to use an object structure instead. I'm checking whether the head commit still has the same payload leak issue that was flagged in the prior run. The commits are different—the previous one was at c868709 while head is now at 3f61538—so I need to verify if the code that logs the PersistenceDecodeError message has changed since then. Looking at the diff, the current code logs only .message rather than the whole error object, which suggests the concern may have already been addressed. But I need to verify that the .message field itself is actually sanitized and doesn't leak the raw payload by checking the PersistenceDecodeError definition and how fromSchemaError constructs its message. The previous commit got rebased away, so I'm shifting focus to what's currently at head. I need to check whether logging is exposing sensitive payload data, specifically by examining how PersistenceDecodeError handles schema errors and what gets included in the error message. I need to check the Errors.ts file to see how fromSchemaError works and whether the .message property exposes any sensitive payload information. The current implementation only logs the .message field, which contains the operation and a sanitized issue summary from summarizeSchemaIssue — not the full error payload. This means the earlier concern about serializing the cause field has already been addressed by the code at head. — re-exposing the raw decoded row. So the prior comment was about logging the entire PersistenceDecodeError object, and the fix was to log only .message instead. The current code at head logs just the sanitized message with operation and issue tags, which resolves the original leak concern. Now I need to verify this against the requirements. Looking at the Effect conventions being used here: the imports follow the namespace subpath pattern correctly, and Effect.catch is the right choice since the entire error channel from decodeRuntimeRow is being handled uniformly. The Arr.filterMap with Result.succeed/Result.failVoid is a bit unusual since filterMap typically expects Option, but I need to check if this is valid in the current Effect version. I'm getting sidetracked on the filterMap implementation details, but that's not really relevant to the Effect service convention check. The actual concern here is whether the error and logging follow the convention about keeping sanitized errors bounded — specifically, not reintroducing raw wire payloads into fields like detail, reason, or message. Let me look at what the current log annotation actually contains. The log is passing threadId as a correlation identifier and the error's .message property, which only contains sanitized tag names from the decode error. That's bounded and safe, no violation there. But now I'm second-guessing whether the second argument to Effect.logWarning is even the right shape for an annotation — I should verify how that function actually accepts its arguments. Looking at the convention text, it mentions Effect.catch as valid for handling intentional errors, so that exists. The real question is whether decodeRuntimeRow's error channel — a SchemaError — qualifies as a tagged failure that should use Effect.catchTags instead. Since the entire error channel is a decode failure and they're handling all of them by skipping, using Effect.catch for the whole channel is the right approach. I should also verify that logging just .message is safe — checking summarizeSchemaIssue, it only returns the _tag and recurses without including any values, so that's secure. The error field stores just the string message, not the full error object. Looking back at the prior comment about logging the whole PersistenceDecodeError leaking information, it seems the fix was to log only .message, which is exactly what the current code does. So that concern appears to be resolved. Now I should check whether there are any other Effect service convention violations in the modified code, particularly verifying that the imports at the top of the file follow the namespace subpath import pattern. The imports look good with namespace subpath conventions being followed. I'm checking that the earlier concern about logging the full error object has been addressed—the code now properly logs only the .message property rather than the entire error, which keeps sensitive information sanitized. Looking at the test file changes, those are fine since tests have more flexibility with explicit service passing. The fromSchemaError factory method isn't a violation either since it does real normalization work beyond just wrapping the constructor. I think the review is complete—the flagged issue has been resolved by restricting what gets logged. Now I need to format the response. The check instructions say to output "All clear" when there are no findings, but the system requires JSON output. I'll set the state to success with an appropriate title and summary reflecting that no issues remain.


Agent Credits: 48 credits