Use Effect schemas in diagnostics parsing#2752
Draft
cursor[bot] wants to merge 1 commit into
Draft
Conversation
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Comment on lines
+55
to
+63
| const WindowsProcessRecordJson = Schema.Struct({ | ||
| ProcessId: Schema.optional(Schema.Number), | ||
| ParentProcessId: Schema.optional(Schema.Number), | ||
| Name: Schema.optional(Schema.String), | ||
| CommandLine: Schema.optional(Schema.String), | ||
| Status: Schema.optional(Schema.String), | ||
| WorkingSetSize: Schema.optional(Schema.Number), | ||
| PercentProcessorTime: Schema.optional(Schema.Number), | ||
| }); |
Contributor
There was a problem hiding this comment.
🟠 High diagnostics/ProcessDiagnostics.ts:55
WindowsProcessRecordJson uses Schema.optional, which accepts string | undefined, but PowerShell outputs JSON null for fields like Status and CommandLine. JavaScript null fails validation, causing decodeWindowsProcessJson to return Option.none() and parseWindowsProcessRows to silently return an empty array instead of the process list. Consider using Schema.NullOr(Schema.String) for fields that can be null.
const WindowsProcessRecordJson = Schema.Struct({
- ProcessId: Schema.optional(Schema.Number),
- ParentProcessId: Schema.optional(Schema.Number),
- Name: Schema.optional(Schema.String),
- CommandLine: Schema.optional(Schema.String),
- Status: Schema.optional(Schema.String),
- WorkingSetSize: Schema.optional(Schema.Number),
- PercentProcessorTime: Schema.optional(Schema.Number),
+ ProcessId: Schema.NullOr(Schema.Number),
+ ParentProcessId: Schema.NullOr(Schema.Number),
+ Name: Schema.NullOr(Schema.String),
+ CommandLine: Schema.NullOr(Schema.String),
+ Status: Schema.NullOr(Schema.String),
+ WorkingSetSize: Schema.NullOr(Schema.Number),
+ PercentProcessorTime: Schema.NullOr(Schema.Number),
});🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/diagnostics/ProcessDiagnostics.ts around lines 55-63:
`WindowsProcessRecordJson` uses `Schema.optional`, which accepts `string | undefined`, but PowerShell outputs JSON `null` for fields like `Status` and `CommandLine`. JavaScript `null` fails validation, causing `decodeWindowsProcessJson` to return `Option.none()` and `parseWindowsProcessRows` to silently return an empty array instead of the process list. Consider using `Schema.NullOr(Schema.String)` for fields that can be null.
Evidence trail:
- apps/server/src/diagnostics/ProcessDiagnostics.ts lines 55-63 (WindowsProcessRecordJson schema using Schema.optional)
- apps/server/src/diagnostics/ProcessDiagnostics.ts lines 69-71 (decodeWindowsProcessJson using Schema.decodeUnknownOption)
- apps/server/src/diagnostics/ProcessDiagnostics.ts lines 203-208 (parseWindowsProcessRows returning [] on Option.none)
- apps/server/src/diagnostics/ProcessDiagnostics.ts lines 377-383 (PowerShell command using Get-CimInstance Win32_Process and ConvertTo-Json)
- package.json line 12: effect version 4.0.0-beta.59
- Effect Schema v4 migration guide (https://github.com/Effect-TS/effect-smol/blob/main/migration/schema.md): `{ nullable: true }` → `optional(NullOr(schema))` confirms Schema.optional does not accept null
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
effect/Durationvalue.Schema.fromJsonStringdecoding andOption-based row normalization.JSON.parseto a hoisted schema JSON decoder.ChildProcessSpawnermock layer.Why
These changes make diagnostics parsing more idiomatic Effect: policies are represented with
Duration, JSON decoding flows througheffect/Schema, and optional normalization avoids sentinel nulls internally while preserving the external contract shape.UI Changes
None.
Checklist
Note
Replace manual JSON parsing with Effect schemas in diagnostics parsing
WindowsProcessRecordJsonschema in ProcessDiagnostics.ts to decode Windows process output (single record or array) viaSchema.decodeUnknownOption, replacing ad-hoc parsing.normalizeWindowsProcessRowto accept typed schema output and returnOption<ProcessRow>, clamping negative CPU/memory to zero, defaulting missingStatusto'Live', and falling back toNamewhenCommandLineis empty.JSON.parsetry/catch in TraceDiagnostics.ts withdecodeTraceRecordJsonusingSchema.decodeUnknownOption; invalid lines still incrementparseErrorCount.PROCESS_QUERY_TIMEOUT_MSconstant with aDuration-basedPROCESS_QUERY_TIMEOUT.📊 Macroscope summarized d6708e4. 2 files reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues