fix(server): omit relations to sliced-away models from OpenAPI spec

[ymc9]

Summary

Fixes #2628.

When a model is excluded via slicing.excludedModels (or not in includedModels), the RPC OpenAPI generator still emitted a $ref for relation fields on other models that point to the excluded model. For example, with Workspace excluded, the Link entity schema still contained:

"workspace": { "$ref": "#/components/schemas/Workspace" }

Since the Workspace schema is (correctly) not generated, this is a dangling $ref — it leaks the excluded model and makes the generated spec structurally invalid (it even fails OpenAPI validation).

Fix

RPCApiSpecGenerator.buildModelEntitySchema now skips relation fields whose target model is not included by the slicing options, mirroring the guard the REST generator already applies in its relation-emitting paths.

Note: per the issue scope this addresses relation inclusion only; FK scalar fields (e.g. workspaceId) are intentionally left unchanged for now.

Test

Added a test under RPC OpenAPI spec generation - queryOptions slicing that excludes Post and asserts the referencing User/Comment entity schemas keep their scalar fields but drop the posts/post relations. Because generateSpec validates the document, the previously dangling $ref made this fail before the fix. Verified red→green.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed OpenAPI spec generation to avoid invalid references when models are excluded. Relation fields targeting omitted models are skipped and procedure responses that would reference excluded models now emit safe generic response schemas.

• Tests

  • Added tests validating that excluded models are removed from entity schemas and that procedure responses fall back to generic shapes.

[claude]

Claude finished @ymc9's task —— View job

---

Code Review Complete ✅

• Read and analyze the implementation changes in openapi.ts
• Review the test coverage additions
• Assess code quality and best practices
• Check for potential bugs or security concerns
• Evaluate performance considerations
• Provide comprehensive feedback

Review Summary

This is a well-implemented bug fix that addresses a specific issue with OpenAPI spec generation when models are sliced away. The fix correctly handles dangling $ref entries.

Code Quality & Best Practices ⭐

Excellent implementation - The fix follows several good practices:

1. Consistent with existing patterns: Uses the same isModelIncluded utility that the REST generator already employs, ensuring consistency across the codebase
2. Clear documentation: The inline comment explains exactly what the guard prevents and why it's necessary
3. Minimal change scope: Only touches the specific problematic code path without affecting other functionality
4. Import organization: Properly imports the utility function from the existing spec-utils module

The added code at packages/server/src/api/rpc/openapi.ts:689-691:

// Skip relations pointing to a model that's sliced away — otherwise we'd emit
// a dangling `$ref` to a schema that's not in the spec.
if (!isModelIncluded(fieldDef.type, this.queryOptions)) continue;

Test Coverage ⭐

Comprehensive test coverage with the new test case at lines 599-617:

1. Validation testing: The test leverages the existing generateSpec function which calls OpenAPI validation, ensuring the fix actually resolves structural validity
2. Multi-model verification: Tests both User and Comment models to ensure the fix works across different referencing scenarios
3. Positive assertions: Confirms that non-relation fields are preserved while relation fields to excluded models are omitted
4. Clear test intent: Test name and comments clearly explain what's being validated

Security Considerations ✅

No security concerns identified:

• The fix prevents information leakage by properly omitting references to excluded models
• Uses existing, trusted utility functions
• No introduction of new attack vectors

Performance Considerations ✅

Minimal performance impact:

• The isModelIncluded check is a simple lookup operation
• Only executes for relation fields, which are typically a small subset of all fields
• Consistent with existing performance characteristics of the REST generator

Potential Issues & Suggestions

Minor Considerations:

1. Edge case handling: The fix assumes fieldDef.type accurately represents the target model name. This appears consistent with the codebase patterns, but worth noting for future maintenance.

2. Consistency verification: The fix mirrors the REST generator's approach, which is excellent for maintainability. Consider whether this logic could be extracted to a shared utility to prevent future divergence.

Final Assessment

This is a high-quality bug fix that:

• ✅ Correctly addresses the stated problem
• ✅ Follows established patterns and conventions
• ✅ Includes proper test coverage with validation
• ✅ Has clear documentation and intent
• ✅ Maintains backward compatibility
• ✅ Introduces no security or performance concerns

The implementation demonstrates good software engineering practices and should be safe to merge.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6bdf3164-3313-4473-b91f-02989e433355

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9f860 and 0d393a4.

📒 Files selected for processing (2)

• packages/server/src/api/rpc/openapi.ts
• packages/server/test/openapi/rpc-openapi.test.ts

---

📝 Walkthrough

Walkthrough

OpenAPI schema generation now omits relation properties whose target models are excluded and makes procedure response schemas fall back to generic shapes when their return model is excluded, avoiding dangling $ref references. Tests were added to verify both behaviors.

Changes

OpenAPI Excluded Model Filtering

Layer / File(s)	Summary
Schema relation filtering for excluded models packages/server/src/api/rpc/openapi.ts	Import isModelIncluded and guard relation field generation in buildModelEntitySchema: skip relation properties whose target model is excluded to prevent dangling $ref references.
Procedure response schema fallback packages/server/src/api/rpc/openapi.ts	In getProcedureDataSchema, when a procedure return type is an excluded model, emit a generic schema ({}) or an array of {} for lists instead of a $ref to a missing component.
Excluded model relation removal & procedure fallback tests packages/server/test/openapi/rpc-openapi.test.ts	New tests: one asserts User and Comment schemas drop relation fields pointing to excluded Post; another verifies the User component is absent when excluded and procedures returning User use generic data schemas to avoid dangling $refs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit nudges schemas neat and sound, 🐇
Hiding links where excluded models are bound,
Procedures still answer, but shy from refs,
Empty shapes stand guard like tiny chefs,
Tests hop in to prove the garden's sound.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	The PR addresses the OpenAPI schema generation aspect of issue #2628, but the issue also requires runtime query enforcement to prevent excluded models' FK and relation fields from being queryable or present in responses.	Verify that runtime query enforcement has been implemented separately or confirm this PR is part of a broader fix addressing both OpenAPI and runtime aspects.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: omitting relation fields to excluded models from the OpenAPI spec generation.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing OpenAPI spec generation for sliced models, with no unrelated modifications introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-2628-openapi-sliced-relations

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

packages/server/test/openapi/rpc-openapi.test.ts (1)

599-617: ⚡ Quick win

Consider verifying that FK scalar fields remain present.

The test validates that relation fields (posts, post) are removed, but doesn't verify that foreign-key scalar fields like Comment.postId remain intact. Since the PR summary explicitly states "Foreign-key scalar fields (e.g., workspaceId) are intentionally unchanged," adding an assertion would document this design decision and prevent regressions.

🧪 Suggested enhancement to verify FK scalar field preservation

         const commentProps = (spec.components?.schemas?.['Comment'] as any)?.properties;
         expect(commentProps?.['content']).toBeDefined();
         expect(commentProps?.['post']).toBeUndefined();
+        // FK scalar fields should remain even when the relation is dropped
+        expect(commentProps?.['postId']).toBeDefined();
     });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/test/openapi/rpc-openapi.test.ts` around lines 599 - 617, Add
assertions to the test that verify foreign-key scalar fields are preserved:
after generating spec with generateSpec(handler) inspect
spec.components?.schemas?['Comment'] and assert that the scalar FK property
(e.g., 'postId') is still defined (and similarly any other FK scalars you
expect, like workspaceId); keep the existing checks that relation properties
('posts', 'post') are removed. Locate this logic around the existing test using
createTestClient, RPCApiHandler and generateSpec to add the new expect(...)
assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/server/test/openapi/rpc-openapi.test.ts`:
- Around line 599-617: Add assertions to the test that verify foreign-key scalar
fields are preserved: after generating spec with generateSpec(handler) inspect
spec.components?.schemas?['Comment'] and assert that the scalar FK property
(e.g., 'postId') is still defined (and similarly any other FK scalars you
expect, like workspaceId); keep the existing checks that relation properties
('posts', 'post') are removed. Locate this logic around the existing test using
createTestClient, RPCApiHandler and generateSpec to add the new expect(...)
assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3f5fa5de-e260-4094-a045-7aa03fd9dd6b

📥 Commits

Reviewing files that changed from the base of the PR and between 431ad02 and 0f9f860.

📒 Files selected for processing (2)

• packages/server/src/api/rpc/openapi.ts
• packages/server/test/openapi/rpc-openapi.test.ts

[claude]

Claude finished @ymc9's task —— View job

---

Code Review Complete ✅

• Read and analyze the implementation changes in openapi.ts
• Review the test coverage additions
• Assess code quality and best practices
• Check for potential bugs or security concerns
• Evaluate performance considerations
• Provide comprehensive feedback

Review Summary

This is a well-implemented bug fix that addresses a specific issue with OpenAPI spec generation when models are sliced away. The fix correctly handles dangling $ref entries.

Code Quality & Best Practices ⭐

Excellent implementation - The fix follows several good practices:

1. Consistent with existing patterns: Uses the same isModelIncluded utility that the REST generator already employs, ensuring consistency across the codebase
2. Clear documentation: The inline comment explains exactly what the guard prevents and why it's necessary
3. Minimal change scope: Only touches the specific problematic code path without affecting other functionality
4. Import organization: Properly imports the utility function from the existing spec-utils module

The added code at packages/server/src/api/rpc/openapi.ts:689-691:

// Skip relations pointing to a model that's sliced away — otherwise we'd emit
// a dangling `$ref` to a schema that's not in the spec.
if (!isModelIncluded(fieldDef.type, this.queryOptions)) continue;

Test Coverage ⭐

Comprehensive test coverage with the new test case at lines 599-617:

1. Validation testing: The test leverages the existing generateSpec function which calls OpenAPI validation, ensuring the fix actually resolves structural validity
2. Multi-model verification: Tests both User and Comment models to ensure the fix works across different referencing scenarios
3. Positive assertions: Confirms that non-relation fields are preserved while relation fields to excluded models are omitted
4. Clear test intent: Test name and comments clearly explain what's being validated

Security Considerations ✅

No security concerns identified:

• The fix prevents information leakage by properly omitting references to excluded models
• Uses existing, trusted utility functions
• No introduction of new attack vectors

Performance Considerations ✅

Minimal performance impact:

• The isModelIncluded check is a simple lookup operation
• Only executes for relation fields, which are typically a small subset of all fields
• Consistent with existing performance characteristics of the REST generator

Potential Issues & Suggestions

Minor Considerations:

1. Edge case handling: The fix assumes fieldDef.type accurately represents the target model name. This appears consistent with the codebase patterns, but worth noting for future maintenance.

2. Consistency verification: The fix mirrors the REST generator's approach, which is excellent for maintainability. Consider whether this logic could be extracted to a shared utility to prevent future divergence.

Final Assessment

This is a high-quality bug fix that:

• ✅ Correctly addresses the stated problem
• ✅ Follows established patterns and conventions
• ✅ Includes proper test coverage with validation
• ✅ Has clear documentation and intent
• ✅ Maintains backward compatibility
• ✅ Introduces no security or performance concerns

The implementation demonstrates good software engineering practices and should be safe to merge.

---