[codex] Structure persistence error correlation#3439
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unbounded list decode correlation
- Removed
{ concurrency: "unbounded" }fromProviderSessionRuntimeRepository.list'sEffect.forEachso decode errors are reported sequentially in query sort order, making error correlation deterministic and consistent withAuthSessions.listActive.
- Removed
Or push these changes by commenting:
@cursor push 9f31b83ea5
Preview (9f31b83ea5)
diff --git a/apps/server/src/persistence/ProviderSessionRuntime.ts b/apps/server/src/persistence/ProviderSessionRuntime.ts
--- a/apps/server/src/persistence/ProviderSessionRuntime.ts
+++ b/apps/server/src/persistence/ProviderSessionRuntime.ts
@@ -280,19 +280,16 @@
),
),
Effect.flatMap((rows) =>
- Effect.forEach(
- rows,
- (row) =>
- decodeRuntimeRow(row).pipe(
- Effect.mapError((cause) =>
- PersistenceDecodeError.fromSchemaError(
- "ProviderSessionRuntimeRepository.list:decodeRows",
- cause,
- { threadId: row.threadId },
- ),
+ Effect.forEach(rows, (row) =>
+ decodeRuntimeRow(row).pipe(
+ Effect.mapError((cause) =>
+ PersistenceDecodeError.fromSchemaError(
+ "ProviderSessionRuntimeRepository.list:decodeRows",
+ cause,
+ { threadId: row.threadId },
),
),
- { concurrency: "unbounded" },
+ ),
),
),
);You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 1f0f376. Configure here.
ApprovabilityVerdict: Approved Adds correlation IDs to persistence errors for debugging/tracing without changing runtime behavior. The changes are mechanical additions of error metadata with comprehensive test coverage. You can customize Macroscope's approvability policy. Learn more. |
Dismissing prior approval to re-evaluate ba1abed
ba1abed to
d305837
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
d305837 to
2582cf0
Compare
Co-authored-by: codex <codex@users.noreply.github.com>


Summary
Why
Persistence failures identified the repository operation but often could not be tied back to the affected session, pairing link, or provider thread. This adds safe correlation without recording credentials, tokens, subjects, payloads, or timestamps.
Safety
The correlation schema permits exactly one of
sessionId,currentSessionId,pairingLinkId, orthreadId. Pairing-link credential-based reads intentionally remain uncorrelated. Error messages continue to derive only from the existing operation/detail/issue fields.Validation
vp test run apps/server/src/persistence(14 files, 22 tests)vp test run apps/server/src/persistence/Errors.test.ts apps/server/src/persistence/RepositoryErrorCorrelation.test.ts(2 files, 5 tests)vp check(0 errors; 20 existing warnings)vp run typecheckNote
Low Risk
Observability and error-shaping only; success paths and public error message format are unchanged, with correlation limited to non-secret identifiers.
Overview
Adds a bounded
PersistenceErrorCorrelationshape (sessionId,currentSessionId,pairingLinkId, orthreadId) onPersistenceSqlErrorandPersistenceDecodeError, so failures can be tied to an entity without putting credentials, subjects, or payloads in messages.Auth sessions, pairing links, and provider session runtime repositories now read
Unknownraw SQL rows and decode in a second step. That lets per-row decode failures carry the row’s id (e.g. corruptlistActive/listdata). SQL and encode-path errors get correlation where the input already has a safe id; pairing-link lookups by credential still omit correlation on the query error path.New
RepositoryErrorCorrelation.test.tsintegration tests assert correlation fields are set and that error text does not leak sensitive values.Reviewed by Cursor Bugbot for commit 0e3803e. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add correlation fields to persistence errors in auth and runtime repositories
PersistenceErrorCorrelationunion type to Errors.ts, allowingPersistenceSqlErrorandPersistenceDecodeErrorto carry an optional correlation payload (e.g.sessionId,pairingLinkId,threadId).AuthSessions,AuthPairingLinks, andProviderSessionRuntimerepositories to return raw rows (unknown-typed fields), then decode them explicitly post-query so decode errors can be correlated to the offending row's ID.Macroscope summarized 0e3803e.