Sanitize the definition token in nested JsonSchema references

[fubhy]

Summary

A nested $defs reference can retain an invalid unsanitized definition name while the component is stored under a sanitized key, producing a dangling OpenAPI reference.

Important

This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.

Nested definition references sanitize the wrong token

Module: JsonSchema
Audit ID: core-g-r-jsonschema-nested-reference-wrong-token
Severity / confidence: medium / high

What happens

A nested $defs reference can retain an invalid unsanitized definition name while the component is stored under a sanitized key, producing a dangling OpenAPI reference.

Why it happens

The rewrite sanitizes tokens[tokens.length - 1], which is the final nested path token rather than the definition token. For #/$defs/A$B/properties/value it checks value, emits A$B in the reference, and stores the component as A_B.

Expected behavior

Every #/$defs//... reference is rewritten under the sanitized OpenAPI component key while preserving the remaining pointer path.

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

• packages/effect/src/JsonSchema.ts:678-689

View problematic code at packages/effect/src/JsonSchema.ts:678-689

  function rewrite(schema: JsonSchema): JsonSchema {
    return rewrite_refs(schema, ($ref) => {
      const tokens = $ref.split("/")
      if (tokens.length > 0) {
        const identifier = unescapeToken(tokens[tokens.length - 1])
        const sanitized = keyMap.get(identifier)
        if (sanitized !== undefined) {
          $ref = tokens.slice(0, -1).join("/") + "/" + sanitized
        }
      }
      return $ref.replace(RE_DEFS, "#/components/schemas")
    }) as JsonSchema

View exact lines on GitHub

Reproduction

pnpm vitest run packages/effect/test/JsonSchema.test.ts -t "sanitizes the definition token in nested references"

Observed failure: The intended failure was reproduced: the emitted reference retained A$B instead of A_B.

Implementation handoff

The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.

1. Start with the pinned implementation excerpts and the Why it happens analysis above.
2. Change the implementation so it satisfies the stated Expected behavior; do not weaken or remove the reproduction assertions.
3. Run the focused reproduction command(s) and confirm the observed failures become passing tests:

pnpm vitest run packages/effect/test/JsonSchema.test.ts -t "sanitizes the definition token in nested references"

4. Run the affected package's existing tests, then the repository lint and type checks before requesting review.

Audit provenance

• Audit base: c9b56ab507f224426ee8388dc450da447ec4715f
• Reproduction base: c9b56ab507f224426ee8388dc450da447ec4715f
• Findings: core-g-r-jsonschema-nested-reference-wrong-token
• Initial patch: focused reproduction tests; implementation fix pending

[changeset-bot]

⚠️ No Changeset found

Latest commit: cf3f6ce

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[pullfrog]

Important

The new regression test is correct and currently failing because the implementation fix it exercises is not yet present on this branch. Merge as-is will break CI.

Reviewed changes

• Reviewed the focused regression test added to packages/effect/test/JsonSchema.test.ts.
• Confirmed the test reproduces the bug described in the PR by running the targeted Vitest filter; it fails as expected with the existing source.

⚠️ Implementation fix missing

The diff adds the regression test, but packages/effect/src/JsonSchema.ts still contains the original bug at lines 678–689. The rewrite helper looks up tokens[tokens.length - 1], so for #/$defs/A$B/properties/value it sanitizes the trailing value token instead of the definition token A$B, producing #/components/schemas/A$B/properties/value.

Merging this branch leaves CI red. The source needs to sanitize the token immediately following $defs in the JSON Pointer, preserving the rest of the path.

Technical details

# Fix nested `$defs` reference sanitization

## Affected sites
- `packages/effect/src/JsonSchema.ts:678-689` — `rewrite` sanitizes the last pointer token (`tokens[tokens.length - 1]`) instead of the definition token.

## Required outcome
- For every `#/$defs/<definition>/...` reference, the `<definition>` token must be rewritten to its sanitized OpenAPI component key while the remaining pointer path is preserved.
- The added test `sanitizes the definition token in nested references` must pass.

## Suggested approach
- Split the `$ref`, locate the `$defs` segment, and sanitize the next token using the existing `keyMap`.
- Reconstruct the `$ref` and then apply the existing `#/components/schemas` replacement.

Example for `#/$defs/A$B/properties/value`:
- tokens = `["#", "$defs", "A$B", "properties", "value"]`
- sanitize `"A$B"` → `"A_B"`
- emit `#/components/schemas/A_B/properties/value`

ℹ️ Nitpicks

• The new assertion uses assert.deepStrictEqual from @effect/vitest, but the file already imports deepStrictEqual from @effect/vitest/utils. Prefer the existing import for consistency.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

This imports assert only for the new assert.deepStrictEqual call. The file already imports deepStrictEqual from @effect/vitest/utils, so prefer that existing utility for consistency.