Fix readWrite transform for schemas with all readOnly/writeOnly properties #3213
Conversation
|
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…properties
The fix removes the requirement for schemas to have the 'normal' scope in order to be split. Now schemas with all properties marked as readOnly or writeOnly will be correctly split into read and write variants.
Changes:
- Removed the `!nodeInfo.scopes?.has('normal')` condition from the schema splitting logic
- Updated the dependency check to not require 'normal' scope
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Schemas with all readOnly or writeOnly properties now correctly generate split variants: - QuxAllRead split into ReadableQuxAllRead and WritableQuxAllRead - QuxAllWrite split into ReadableQuxAllWrite and WritableQuxAllWrite - ModelWithNestedProperties now generates ModelWithNestedPropertiesWritable Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
|
@copilot All those objects with unknown keys are undesirable in this pull request. If an object had all properties read-only, the write schema should NOT handle an empty object and assume it accepts any properties with unknown values. There's a specific keyword in OpenAPI to express this behavior. If no such keyword exists, as is the case in the affected tests, we should ignore the whole object as we did before. |
Per maintainer feedback, schemas with only readOnly OR only writeOnly properties
should NOT be split. The fix now requires schemas to have BOTH 'read' AND 'write'
scopes (i.e., a mix of readOnly and writeOnly properties) to be split.
This removes the unwanted `{ [key: string]: unknown; }` empty objects that were
being generated for schemas like QuxAllRead and QuxAllWrite.
Behavior:
- TokenObtain (has both readOnly and writeOnly) - STILL SPLITS correctly
- QuxAllRead (only readOnly) - NO LONGER SPLITS (correct)
- QuxAllWrite (only writeOnly) - NO LONGER SPLITS (correct)
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Fixed in commit 32b4ab2. The splitting condition now requires schemas to have BOTH 'read' AND 'write' scopes. Schemas with only readOnly OR only writeOnly properties are no longer split, eliminating the unwanted empty object types with unknown keys. |
|
@copilot Nope, this isn't it. The previous iteration was almost there, go back to it. From there, you need to figure out why those objects weren't being skipped anymore, even though they were originally getting skipped. What's the difference maker, do they now have an orphaned scope? We need to be able to differentiate between objects that were created without any properties and hence allow any property, and objects whose schemas look the same but are a result of us stripping properties. In the latter case, these should be removed from the spec just like before. For example, if an object has only one property Foo and we remove it, that doesn't mean this object now accepts any property. It means this object should be completely removed. |
Check the return value of pruneSchemaByScope to determine if a variant
became empty after removing all scoped properties. If either the read
or write variant would be empty (should be removed), skip splitting
the schema entirely.
This fixes the issue where schemas like QuxAllRead (only readOnly) or
QuxAllWrite (only writeOnly) were being split into variants where one
variant was an unwanted empty object { [key: string]: unknown; }.
Now:
- Schemas with only readOnly properties are NOT split
- Schemas with only writeOnly properties are NOT split
- Schemas with BOTH readOnly and writeOnly properties ARE split
- Empty schemas from pruning are properly removed instead of becoming
objects that accept any properties
Co-authored-by: mrlubos <12529395+mrlubos@users.noreply.github.com>
Fixed in commit b1a970f. The issue was that |
Fix readWrite transform for schemas with all readOnly/writeOnly properties
This PR addresses an issue where schemas with ALL properties marked as
readOnlyorwriteOnlywere being incorrectly split, generating unwanted empty object types.Root Cause
The
pruneSchemaByScopefunction correctly returnstruewhen a schema becomes empty after pruning (indicating it should be removed), but this return value was being ignored. This caused schemas with only readOnly OR only writeOnly properties to be split into variants where one variant was an empty object{ [key: string]: unknown; }.Solution
Check if either variant becomes empty after pruning. If
pruneSchemaByScopereturnstruefor either the read or write variant, skip splitting the schema entirely.This differentiates between:
Behavior
TokenObtain (has both readOnly and writeOnly properties):
✅ Correctly splits into:
TokenObtain(response with readOnly fields)TokenObtainWritable(request with writeOnly fields)QuxAllRead (only readOnly properties):
✅ Does NOT split - kept as single type
QuxAllRead(write variant would be empty after pruning)QuxAllWrite (only writeOnly properties):
✅ Does NOT split - kept as single type
QuxAllWrite(read variant would be empty after pruning)Quux (references QuxAllRead):
✅ Does split - because it references schemas, split into
ReadableQuuxandWritableQuuxChanges
pruneSchemaByScopereturn value to detect empty schemasOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.