Fix/kopai app docs warning#59
Conversation
📝 WalkthroughWalkthroughThis PR fixes an incomplete AttributeValue schema generation issue in fastify-type-provider-zod by introducing a custom schema transformer that injects and properly defines recursive schemas. It also wraps linting tools through pnpm and adds a corresponding changeset entry. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
44-46:⚠️ Potential issue | 🟡 MinorInconsistent
pnpm execwrapping for the*.{json,md,yaml,yml}staged command.Lines 41–42 were updated to use
pnpm execto ensure the locally resolvedprettierbinary is used, but line 45 still invokesprettier --writedirectly. The same PATH-resolution concern applies to JSON/MD/YAML/YML files.🔧 Proposed fix
"*.{json,md,yaml,yml}": [ - "prettier --write" + "pnpm exec prettier --write" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 44 - 46, The staged command for "*.{json,md,yaml,yml}" uses the system `prettier --write` instead of the project-local binary; update the package.json pre-commit/husky/lint-staged entry for the "*.{json,md,yaml,yml}" pattern to invoke the locally resolved prettier the same way other entries do (i.e., wrap the prettier invocation with the project runner used elsewhere, matching the existing lines that use `pnpm exec`), so the pre-commit hook consistently uses the repo’s local prettier.
🧹 Nitpick comments (1)
packages/app/src/server.ts (1)
64-98: Consider the idiomaticfastify-type-provider-zodv6 / Zod v4 fix: register the recursive schema with the global Zod registry.In
fastify-type-provider-zodv6, you register schemas with the global Zod registry and assign them an id;fastifySwaggerthen creates an OpenAPI document that references those named schemas and includes their definitions automatically.Applying that at the point where the recursive
AttributeValueschema is defined would render the entiretransformObjectworkaround unnecessary:// wherever the recursive AttributeValue Zod schema is defined (e.g., routes/types.ts) import { z } from "zod/v4"; const AttributeValue: z.ZodType = z.lazy(() => z.union([ z.string(), z.number(), z.boolean(), z.array(AttributeValue), z.record(z.string(), AttributeValue), ]) ); // Register it so fastify-type-provider-zod produces a stable named $ref z.globalRegistry.add(AttributeValue, { id: "AttributeValue" });Then
transformObjectcan revert to the default:- transformObject: (input) => { - const result = jsonSchemaTransformObject(input); - ... - return patched as ReturnType<typeof jsonSchemaTransformObject>; - }, + transformObject: jsonSchemaTransformObject,This eliminates the string-manipulation fragility, the hardcoded schema body, and the coupling to the internal
schema0name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/server.ts` around lines 64 - 98, The current transformObject workaround patches JSON output to rename schema0 → AttributeValue and inject a hardcoded schema; instead register the recursive Zod schema with the Zod global registry so fastify-type-provider-zod emits a named component automatically: locate the recursive AttributeValue Zod definition (where you currently build the union/ z.lazy() type) and call z.globalRegistry.add(AttributeValue, { id: "AttributeValue" }) (importing z from "zod/v4"), then remove the custom replace/patch logic in transformObject and return the result from jsonSchemaTransformObject unchanged; keep references to jsonSchemaTransformObject and transformObject to find and remove the workaround.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/server.ts`:
- Around line 70-73: The current naive string replacement using
raw.replaceAll("#/components/schemas/schema0",
"#/components/schemas/AttributeValue") is fragile (ties to internal schema
names, misses schema1/schema2, and can replace occurrences in
descriptions/examples); instead parse the Swagger JSON into an object (use
JSON.parse on raw), recursively walk the object to find all $ref values that
match the pattern /^#\/components\/schemas\/schema\d+$/ and remap only those
reference nodes to the intended public schema name
("#/components/schemas/AttributeValue"), then serialize back to string
(JSON.stringify) and assign to renamed; update the logic around the renamed
variable and the replace step to use this structured transformation so changes
in lazy schema numbering or incidental text won't break the output.
- Around line 79-95: The hardcoded patched.components.schemas.AttributeValue
should be derived from the canonical Zod schema to avoid drift: import the
attributeValue Zod schema from packages/core/src/denormalized-signals-zod.ts and
convert it to an OpenAPI/JSON Schema (using your project’s Zod-to-OpenAPI
converter utility such as zodToJsonSchema/zodToOpenAPI or a small adapter) and
assign that result to patched.components.schemas.AttributeValue instead of the
manual anyOf object; if the conversion utility isn't available, at minimum
replace the literal block with a clear comment referencing the source symbol
(attributeValue in denormalized-signals-zod.ts) and add a TODO to convert it
programmatically.
---
Outside diff comments:
In `@package.json`:
- Around line 44-46: The staged command for "*.{json,md,yaml,yml}" uses the
system `prettier --write` instead of the project-local binary; update the
package.json pre-commit/husky/lint-staged entry for the "*.{json,md,yaml,yml}"
pattern to invoke the locally resolved prettier the same way other entries do
(i.e., wrap the prettier invocation with the project runner used elsewhere,
matching the existing lines that use `pnpm exec`), so the pre-commit hook
consistently uses the repo’s local prettier.
---
Nitpick comments:
In `@packages/app/src/server.ts`:
- Around line 64-98: The current transformObject workaround patches JSON output
to rename schema0 → AttributeValue and inject a hardcoded schema; instead
register the recursive Zod schema with the Zod global registry so
fastify-type-provider-zod emits a named component automatically: locate the
recursive AttributeValue Zod definition (where you currently build the union/
z.lazy() type) and call z.globalRegistry.add(AttributeValue, { id:
"AttributeValue" }) (importing z from "zod/v4"), then remove the custom
replace/patch logic in transformObject and return the result from
jsonSchemaTransformObject unchanged; keep references to
jsonSchemaTransformObject and transformObject to find and remove the workaround.
| const renamed = raw.replaceAll( | ||
| "#/components/schemas/schema0", | ||
| "#/components/schemas/AttributeValue" | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all z.lazy() usages to identify unregistered lazy schemas
rg -n --type ts 'z\.lazy\(' -A 3Repository: kopai-app/kopai-mono
Length of output: 11038
🏁 Script executed:
#!/bin/bash
# Get context around lines 70-73 in server.ts to understand the full patch
cat -n packages/app/src/server.ts | sed -n '50,90p'Repository: kopai-app/kopai-mono
Length of output: 1713
🏁 Script executed:
#!/bin/bash
# Search for schema0, schema1, schema2 references in the codebase to understand naming patterns
rg -n 'schema[0-9]' --type ts --type jsonRepository: kopai-app/kopai-mono
Length of output: 336
schema0 is an unstable internal implementation detail — coupling to library internals risks silent breakage.
Three concrete concerns with this approach:
-
Multiple lazy schemas: The codebase contains many
z.lazy()schemas across multiple files. While onlyschema0appears in the current Swagger output, if any route uses an unregisteredz.lazy()that generatesschema1,schema2, etc., those dangling references remain unpatched, producing Swagger validation warnings. -
Library version fragility:
schema0is not part offastify-type-provider-zod's public API. A patch release could change the naming convention (e.g., to a content-hash or UUID), silently breaking this fix with no type error or test failure. -
Overly broad string replacement:
replaceAllon the full JSON string searches the entire serialized document. While#/components/schemas/schema0is fairly specific, it would incorrectly rewrite any occurrence in adescriptionorexamplefield that happens to contain this exact string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/app/src/server.ts` around lines 70 - 73, The current naive string
replacement using raw.replaceAll("#/components/schemas/schema0",
"#/components/schemas/AttributeValue") is fragile (ties to internal schema
names, misses schema1/schema2, and can replace occurrences in
descriptions/examples); instead parse the Swagger JSON into an object (use
JSON.parse on raw), recursively walk the object to find all $ref values that
match the pattern /^#\/components\/schemas\/schema\d+$/ and remap only those
reference nodes to the intended public schema name
("#/components/schemas/AttributeValue"), then serialize back to string
(JSON.stringify) and assign to renamed; update the logic around the renamed
variable and the replace step to use this structured transformation so changes
in lazy schema numbering or incidental text won't break the output.
| patched.components.schemas.AttributeValue = { | ||
| anyOf: [ | ||
| { type: "string" }, | ||
| { type: "number" }, | ||
| { type: "boolean" }, | ||
| { | ||
| type: "array", | ||
| items: { $ref: "#/components/schemas/AttributeValue" }, | ||
| }, | ||
| { | ||
| type: "object", | ||
| additionalProperties: { | ||
| $ref: "#/components/schemas/AttributeValue", | ||
| }, | ||
| }, | ||
| ], | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the AttributeValue Zod schema definition
rg -n 'AttributeValue' --type ts -B 3 -A 10 packages/app/src/Repository: kopai-app/kopai-mono
Length of output: 2348
🏁 Script executed:
#!/bin/bash
# Search for the Zod schema definition of AttributeValue
# Look for z.union, z.discriminatedUnion, z.lazy patterns
rg -n 'z\.(lazy|union|discriminatedUnion)' --type ts -B 2 -A 15 | grep -A 15 -B 2 -i attributeRepository: kopai-app/kopai-mono
Length of output: 29488
🏁 Script executed:
#!/bin/bash
# Alternative: search for where AttributeValue Zod type is defined
# Look for const or type declarations with AttributeValue
rg -n 'const.*AttributeValue|type.*AttributeValue' --type ts -A 10Repository: kopai-app/kopai-mono
Length of output: 4753
🏁 Script executed:
#!/bin/bash
# Verify the mapping by checking both schemas side-by-side
echo "=== Actual Zod schema in denormalized-signals-zod.ts ==="
sed -n '10,17p' packages/core/src/denormalized-signals-zod.ts
echo ""
echo "=== Hardcoded anyOf in server.ts ==="
sed -n '80,94p' packages/app/src/server.tsRepository: kopai-app/kopai-mono
Length of output: 760
Hardcoded anyOf definition creates a maintenance risk if the Zod schema changes.
The hardcoded schema here currently matches the attributeValue Zod definition in packages/core/src/denormalized-signals-zod.ts (string, number, boolean, array, and object types). However, because it is manually authored rather than derived from the Zod definition, any future schema changes will silently break OpenAPI docs without compile-time or runtime warnings.
Consider deriving this schema from the Zod definition instead of maintaining it manually, or add a comment explicitly linking it to the source Zod schema to flag this coupling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/app/src/server.ts` around lines 79 - 95, The hardcoded
patched.components.schemas.AttributeValue should be derived from the canonical
Zod schema to avoid drift: import the attributeValue Zod schema from
packages/core/src/denormalized-signals-zod.ts and convert it to an OpenAPI/JSON
Schema (using your project’s Zod-to-OpenAPI converter utility such as
zodToJsonSchema/zodToOpenAPI or a small adapter) and assign that result to
patched.components.schemas.AttributeValue instead of the manual anyOf object; if
the conversion utility isn't available, at minimum replace the literal block
with a clear comment referencing the source symbol (attributeValue in
denormalized-signals-zod.ts) and add a TODO to convert it programmatically.
Summary by CodeRabbit