Skip to content

[codex] Bound shared schema diagnostics#3424

Merged
juliusmarminge merged 1 commit into
mainfrom
codex/shared-error-diagnostics
Jun 20, 2026
Merged

[codex] Bound shared schema diagnostics#3424
juliusmarminge merged 1 commit into
mainfrom
codex/shared-error-diagnostics

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • preserve structured Schema issue kinds and paths without serializing invalid values
  • bound issue, path, and total diagnostic output; summarize defect and interruption causes without exposing their messages
  • preserve the original Schema issue from lenient JSON parsing instead of stringifying and wrapping it

Validation

  • vp test packages/shared/src/schemaJson.test.ts apps/server/src/sourceControl/GitHubCli.test.ts apps/server/src/sourceControl/GitLabCli.test.ts apps/server/src/sourceControl/AzureDevOpsCli.test.ts (30 tests)
  • vp check (passes with 20 pre-existing warnings)
  • vp run typecheck

Note

Medium Risk
Changes user-visible validation messages across shared JSON/schema helpers used by server and UI boundaries; behavior is safer for secrets but less detailed for debugging.

Overview
formatSchemaError no longer uses Effect Schema’s default formatter (which embedded invalid values). It walks schema issues and emits fixed labels like Invalid type / Invalid value with bounded JSON paths, caps how many issues and how long the string can be, and reports “… and N more issue(s)” when truncated. Defects and interruptions get a count-only summary instead of Cause.pretty messages.

Lenient JSON parse failures now propagate the underlying schema issue instead of wrapping them in InvalidValue with String(error), so malformed input is not echoed in diagnostics.

Tests assert secrets in bad JSON are not present in formatted output.

Reviewed by Cursor Bugbot for commit 3277fa3. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Bound schema diagnostics in formatSchemaError to prevent value exposure

  • Replaces the prior formatSchemaError implementation with a value-agnostic formatter that caps the number of issues, path segment count, path segment length, and total diagnostic length, appending an omitted-issues suffix when truncated.
  • Issue messages are mapped from SchemaIssue tags to stable strings (e.g. 'Invalid type', 'Missing key') so no user-supplied values appear in diagnostics.
  • Fixes lenient JSON parse failures to forward the decoder's inner issue directly instead of wrapping the original input in a new InvalidValue issue.
  • Behavioral Change: formatSchemaError output format changes significantly — callers relying on the previous Schema default formatter output will see different diagnostic strings.

Macroscope summarized 3277fa3.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 97f843a6-bd7f-4568-9162-307fa7493241

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/shared-error-diagnostics

Comment @coderabbitai help to get the list of available commands and usage tips.

@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 Jun 20, 2026

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Filter issues drop nested paths
    • Added a Filter case to collectSchemaDiagnosticIssues that recurses into issue.issue when the inner issue is not a bare InvalidValue, matching Effect's own toDefaultIssues behavior to preserve nested field paths.
  • ✅ Fixed: Omission suffix lost to truncation
    • Changed formatSchemaError to truncate the formatted issues part separately with reserved space for the omission suffix, so the '... and N more issue(s)' line is always preserved regardless of total output length.

Create PR

Or push these changes by commenting:

@cursor push 216bda4d7c
Preview (216bda4d7c)
diff --git a/packages/shared/src/schemaJson.ts b/packages/shared/src/schemaJson.ts
--- a/packages/shared/src/schemaJson.ts
+++ b/packages/shared/src/schemaJson.ts
@@ -78,6 +78,11 @@
   switch (issue._tag) {
     case "Encoding":
       return collectSchemaDiagnosticIssues(issue.issue, path, diagnostics);
+    case "Filter":
+      if (issue.issue._tag !== "InvalidValue") {
+        return collectSchemaDiagnosticIssues(issue.issue, path, diagnostics);
+      }
+      break;
     case "Pointer":
       return collectSchemaDiagnosticIssues(issue.issue, [...path, ...issue.path], diagnostics);
     case "Composite":
@@ -157,11 +162,11 @@
 
   const omittedIssueCount = issueCount - issues.length;
   const formatted = issues.map(formatDiagnosticIssue).join("\n");
-  const withOmittedCount =
-    omittedIssueCount === 0
-      ? formatted
-      : `${formatted}\n... and ${omittedIssueCount} more issue(s)`;
-  return truncateDiagnostic(withOmittedCount, MAX_SCHEMA_DIAGNOSTIC_LENGTH);
+  if (omittedIssueCount === 0) {
+    return truncateDiagnostic(formatted, MAX_SCHEMA_DIAGNOSTIC_LENGTH);
+  }
+  const suffix = `\n... and ${omittedIssueCount} more issue(s)`;
+  return truncateDiagnostic(formatted, MAX_SCHEMA_DIAGNOSTIC_LENGTH - suffix.length) + suffix;
 };
 
 /**

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 034068f. Configure here.

Comment thread packages/shared/src/schemaJson.ts
Comment thread packages/shared/src/schemaJson.ts Outdated
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Security improvement that prevents sensitive values from appearing in schema validation error diagnostics. The changes are self-contained to error formatting with comprehensive test coverage verifying no value leakage.

You can customize Macroscope's approvability policy. Learn more.

Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarminge force-pushed the codex/shared-error-diagnostics branch from 034068f to 3277fa3 Compare June 20, 2026 18:35
@macroscopeapp
macroscopeapp Bot dismissed their stale review June 20, 2026 18:35

Dismissing prior approval to re-evaluate 3277fa3

@juliusmarminge
juliusmarminge merged commit eacceb9 into main Jun 20, 2026
16 checks passed
@juliusmarminge
juliusmarminge deleted the codex/shared-error-diagnostics branch June 20, 2026 18:41
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