Skip to content

Use Effect schemas in diagnostics parsing#2752

Draft
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/effect-codebase-idioms-ff1a
Draft

Use Effect schemas in diagnostics parsing#2752
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/effect-codebase-idioms-ff1a

Conversation

@cursor
Copy link
Copy Markdown
Contributor

@cursor cursor Bot commented May 18, 2026

What Changed

  • Replaced the raw process diagnostics timeout millisecond constant with an effect/Duration value.
  • Switched Windows process diagnostics JSON parsing to hoisted Schema.fromJsonString decoding and Option-based row normalization.
  • Switched trace diagnostics NDJSON line parsing from JSON.parse to a hoisted schema JSON decoder.
  • Added focused coverage for the Windows process diagnostics parser path using the existing ChildProcessSpawner mock layer.

Why

These changes make diagnostics parsing more idiomatic Effect: policies are represented with Duration, JSON decoding flows through effect/Schema, and optional normalization avoids sentinel nulls internally while preserving the external contract shape.

UI Changes

None.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes
Open in Web View Automation 

Note

Replace manual JSON parsing with Effect schemas in diagnostics parsing

  • Introduces WindowsProcessRecordJson schema in ProcessDiagnostics.ts to decode Windows process output (single record or array) via Schema.decodeUnknownOption, replacing ad-hoc parsing.
  • Refactors normalizeWindowsProcessRow to accept typed schema output and return Option<ProcessRow>, clamping negative CPU/memory to zero, defaulting missing Status to 'Live', and falling back to Name when CommandLine is empty.
  • Replaces JSON.parse try/catch in TraceDiagnostics.ts with decodeTraceRecordJson using Schema.decodeUnknownOption; invalid lines still increment parseErrorCount.
  • Replaces the numeric PROCESS_QUERY_TIMEOUT_MS constant with a Duration-based PROCESS_QUERY_TIMEOUT.
📊 Macroscope summarized d6708e4. 2 files reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted

🗂️ Filtered Issues

Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels May 18, 2026
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),
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant