Skip to content

fix(entities): entity delete TOCTOU checks - #951

Merged
ironcommit merged 1 commit into
mainfrom
fix-virtual-model-delete-toctou/rsadler
Jul 29, 2026
Merged

fix(entities): entity delete TOCTOU checks#951
ironcommit merged 1 commit into
mainfrom
fix-virtual-model-delete-toctou/rsadler

Conversation

@ironcommit

@ironcommit ironcommit commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Added optional optimistic-locking deletion via expected_db_version for entities and VirtualModels, including CLI support.
    • Exposed read-only db_version across multiple API schemas to enable version-aware deletes.
  • Bug Fixes
    • Version mismatches now reliably return HTTP 409 Conflict on concurrent modification across deletion and cleanup workflows (including reconciler and file-lock release).
  • Documentation
    • Updated CLI reference and OpenAPI specs to document expected_db_version and the 409 response.
  • Tests
    • Added/updated unit and route tests to cover matching, stale, and conflict scenarios end-to-end.

@ironcommit
ironcommit requested review from a team as code owners July 28, 2026 18:37
@github-actions github-actions Bot added the fix label Jul 28, 2026
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 27612/35363 78.1% 62.6%
Integration Tests 16118/34081 47.3% 19.8%

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Optimistic locking is added to entity deletion. APIs, SDK clients, services, reconcilers, file locks, schemas, CLI documentation, and tests now pass db_version expectations and map stale deletions to conflict responses.

Changes

Optimistic deletion flow

Layer / File(s) Summary
API and schema contracts
openapi/..., packages/nemo_platform_ext/..., packages/nemo_platform_plugin/...
Delete operations accept expected_db_version; entity schemas expose read-only db_version fields; the CLI forwards the new option.
Repository enforcement
services/core/entities/...
Repository deletion validates versions, rolls back stale commits, and maps conflicts to HTTP 409.
Service and cleanup propagation
plugins/..., services/core/models/..., services/core/files/..., plugins/nemo-evaluator/...
Deletion flows fetch or use entity versions, pass them to delete calls, and handle concurrent modification during API and reconciliation paths.
Validation coverage
**/tests/...
Tests cover version forwarding, stale deletion rejection, conflict responses, cleanup behavior, lock release, and updated response shapes.

Suggested labels: fix

Suggested reviewers: benmccown, mikeknep, tylersbray

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 matches the core change: adding TOCTOU/optimistic-lock checks to entity deletes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-virtual-model-delete-toctou/rsadler

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
openapi/ga/individual/platform.openapi.yaml (1)

864-928: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Add the 409 response here. delete_entity_by_name raises HTTPException(status_code=409) on EntityVersionConflictError, so this route should document the stale-version conflict alongside 200/422.

🤖 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 `@openapi/ga/individual/platform.openapi.yaml` around lines 864 - 928, Add a
409 Conflict response to the delete_entity_by_name operation’s responses,
documenting the EntityVersionConflictError stale-version case alongside the
existing 200 and 422 responses, using the appropriate conflict response
description and schema if one is established.
services/core/models/src/nmp/core/models/api/service/prompt_service.py (1)

159-179: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Race: delete() after get() can raise unhandled EntityNotFoundError → 500 instead of 404.

entity_client.delete() is called outside the try/except that only guards get(). If the prompt is deleted between these two calls, the resulting EntityNotFoundError isn't caught here and surfaces as a 500 in prompts.py's endpoint (only EntityConflictError is explicitly handled there). nemo-auditor's and nemo-deployments' equivalent delete handlers avoid this by wrapping both the fetch and the delete in one try block.

🐛 Proposed fix
     async def delete_prompt(self, request: DeletePromptRequest) -> bool:
         """Delete a prompt by workspace and name. Returns False if not found."""
         logger.debug("Deleting prompt", extra={"workspace": request.workspace, "prompt_name": request.name})

         try:
             prompt = await self.entity_client.get(PromptEntity, workspace=request.workspace, name=request.name)
+            await self.entity_client.delete(
+                PromptEntity,
+                prompt.name,
+                workspace=request.workspace,
+                expected_db_version=prompt.db_version,
+            )
         except EntityNotFoundError:
             logger.warning(
                 "Prompt not found for deletion",
                 extra={"workspace": request.workspace, "prompt_name": request.name},
             )
             return False

-        await self.entity_client.delete(
-            PromptEntity,
-            prompt.name,
-            workspace=request.workspace,
-            expected_db_version=prompt.db_version,
-        )
         logger.info("Prompt deleted", extra={"workspace": request.workspace, "prompt_name": request.name})
         return True
🤖 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 `@services/core/models/src/nmp/core/models/api/service/prompt_service.py`
around lines 159 - 179, Update delete_prompt so the try/except
EntityNotFoundError covers both entity_client.get and the subsequent
entity_client.delete calls. Preserve the existing warning and False return when
either operation reports the prompt is missing, while keeping successful
deletion logging and True return unchanged.
🧹 Nitpick comments (2)
services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py (2)

610-622: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Route doesn't declare the new 409 response.

Conflict handling is correct, but unless the @router.delete(...) decorator declares responses={409: {...}}, FastAPI won't emit this in the OpenAPI schema (confirmed missing in openapi.yaml). See consolidated comment for the fix.

🤖 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 `@services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py`
around lines 610 - 622, Update the `@router.delete` decorator for the endpoint
containing `delete_entity_by_name` to declare the `409` response, including an
appropriate response description/schema consistent with the raised
`HTTPException`, so FastAPI includes the conflict response in the OpenAPI
schema.

1-1: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Undocumented 409 on entity delete-by-name. The route now raises HTTPException(409) for version conflicts, but its OpenAPI entry only lists 200/422 — unlike the VirtualModel delete route, which explicitly documents 409.

  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py#L610-622: add responses={409: {"description": "..."}} (or equivalent) to the @router.delete(...) decorator for delete_entity_by_name.
  • openapi/openapi.yaml#L906-928: regenerate after the decorator fix so the 409 response appears alongside 200/422.
🤖 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 `@services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py` at
line 1, Update the delete_entity_by_name `@router.delete` decorator to document
the existing HTTPException(409) version-conflict response alongside the current
200 and 422 responses, matching the VirtualModel delete route’s response
documentation. Regenerate the OpenAPI specification so the generated
delete-by-name schema includes the 409 response.
🤖 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.

Inline comments:
In `@plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py`:
- Around line 183-189: Update the agent deletion flow around
entity_client.delete so it atomically validates that no deployment references
the agent before deletion, preventing a deployment created after the list check
from orphaning it. Alternatively, ensure deployment creation advances the
agent’s db_version so expected_db_version conflicts with concurrent deployment
creation; preserve optimistic concurrency behavior.

In `@plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py`:
- Around line 88-89: Update the fake delete implementation in async delete to
persist each entity’s version and validate expected_db_version before removal;
reject stale or mismatched versions with the same conflict behavior used by the
service. Add a test covering deletion with an outdated expected_db_version,
ensuring MetricService forwards the guard and the entity remains stored.

In
`@services/core/models/src/nmp/core/models/api/service/model_provider_service.py`:
- Around line 299-304: Reorder the provider deletion flow so the conditional
delete via entity_client.delete completes before invoking
_cleanup_model_entity_references(). Preserve the existing cleanup behavior after
a successful delete, and ensure a stale provider.db_version leaves linked model
references unchanged when deletion raises a 409.

---

Outside diff comments:
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 864-928: Add a 409 Conflict response to the delete_entity_by_name
operation’s responses, documenting the EntityVersionConflictError stale-version
case alongside the existing 200 and 422 responses, using the appropriate
conflict response description and schema if one is established.

In `@services/core/models/src/nmp/core/models/api/service/prompt_service.py`:
- Around line 159-179: Update delete_prompt so the try/except
EntityNotFoundError covers both entity_client.get and the subsequent
entity_client.delete calls. Preserve the existing warning and False return when
either operation reports the prompt is missing, while keeping successful
deletion logging and True return unchanged.

---

Nitpick comments:
In `@services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py`:
- Around line 610-622: Update the `@router.delete` decorator for the endpoint
containing `delete_entity_by_name` to declare the `409` response, including an
appropriate response description/schema consistent with the raised
`HTTPException`, so FastAPI includes the conflict response in the OpenAPI
schema.
- Line 1: Update the delete_entity_by_name `@router.delete` decorator to document
the existing HTTPException(409) version-conflict response alongside the current
200 and 422 responses, matching the VirtualModel delete route’s response
documentation. Regenerate the OpenAPI specification so the generated
delete-by-name schema includes the 409 response.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2d10347b-4b63-4370-adb9-b7f033dd51e6

📥 Commits

Reviewing files that changed from the base of the PR and between 48a2614 and bb95212.

⛔ Files ignored due to path filters (14)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/entities/entities.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/entities/entity_delete_entity_by_name_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/guardrail/guardrail_config.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model_delete_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_step.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_task.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/inference/test_virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_entities.py is excluded by !sdk/**
📒 Files selected for processing (79)
  • docs/cli/reference.mdx
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/inference/virtual_models.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/entities.py
  • packages/nmp_common/tests/entities/test_client.py
  • plugins/example-plugin/src/nemo_example_plugin/middleware_service.py
  • plugins/example-plugin/src/nemo_example_plugin/service.py
  • plugins/example-plugin/tests/test_service.py
  • plugins/nemo-agents/openapi/openapi.yaml
  • plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
  • plugins/nemo-agents/tests/unit/test_agents_api.py
  • plugins/nemo-agents/tests/unit/test_runner_deployments.py
  • plugins/nemo-auditor/openapi/openapi.yaml
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/configs.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/targets.py
  • plugins/nemo-auditor/tests/test_api_configs.py
  • plugins/nemo-auditor/tests/test_api_targets.py
  • plugins/nemo-deployments/openapi/openapi.yaml
  • plugins/nemo-deployments/src/nemo_deployments_plugin/api/v2/deployment_configs.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/volume_reconciler.py
  • plugins/nemo-deployments/tests/unit/test_api_deployment_configs.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/metric_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/result_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/results.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py
  • plugins/nemo-evaluator/tests/api/service/test_metric_service.py
  • plugins/nemo-evaluator/tests/api/service/test_result_service.py
  • plugins/nemo-evaluator/tests/api/service/test_task_service.py
  • plugins/nemo-evaluator/tests/api/service/test_taskset_service.py
  • plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_results_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py
  • plugins/nemo-insights/src/nemo_insights_plugin/service.py
  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py
  • services/core/entities/src/nmp/core/entities/app/repository/entity.py
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/entity.py
  • services/core/entities/tests/repository/test_entity_delete_versioning.py
  • services/core/files/src/nmp/core/files/app/file_lock.py
  • services/core/files/tests/test_file_lock.py
  • services/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/virtual_models.py
  • services/core/inference-gateway/tests/unit/test_virtual_models_router.py
  • services/core/models/src/nmp/core/models/api/service/adapter_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_config_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_service.py
  • services/core/models/src/nmp/core/models/api/service/model_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_provider_service.py
  • services/core/models/src/nmp/core/models/api/service/prompt_service.py
  • services/core/models/src/nmp/core/models/api/v2/adapters.py
  • services/core/models/src/nmp/core/models/api/v2/deployment_configs.py
  • services/core/models/src/nmp/core/models/api/v2/deployments.py
  • services/core/models/src/nmp/core/models/api/v2/models.py
  • services/core/models/src/nmp/core/models/api/v2/prompts.py
  • services/core/models/src/nmp/core/models/api/v2/providers.py
  • services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
  • services/core/models/src/nmp/core/models/controllers/provider_reconciler.py
  • services/core/models/tests/unit/api/test_deployment_configs_api.py
  • services/core/models/tests/unit/api/test_deployments_api.py
  • services/core/models/tests/unit/api/test_models_api.py
  • services/core/models/tests/unit/api/test_prompts_api.py
  • services/core/models/tests/unit/api/test_providers_api.py
  • services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
  • services/core/models/tests/unit/controllers/test_provider_reconciler.py
  • services/core/models/tests/unit/test_model_deployment_config_service_unit.py
  • services/core/models/tests/unit/test_model_deployment_service_unit.py
  • services/core/models/tests/unit/test_model_entity_service_unit.py
  • services/core/models/tests/unit/test_model_provider_service_unit.py
  • services/core/models/tests/unit/test_prompt_service_unit.py
  • services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py
  • services/hello-world/src/nmp/hello_world/api/v1/messages/endpoints.py

Comment thread plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py Outdated
Comment thread plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py Outdated
@ironcommit
ironcommit force-pushed the fix-virtual-model-delete-toctou/rsadler branch from bb95212 to ad0ce55 Compare July 28, 2026 20:09

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
plugins/nemo-evaluator/tests/api/service/test_metric_service.py (1)

89-99: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Align the metric delete fake with the task/taskset fakes — reject mismatched expected_db_version and add a stale-version delete_metric test so a version-check regression is caught.

🤖 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 `@plugins/nemo-evaluator/tests/api/service/test_metric_service.py` around lines
89 - 99, Update the fake metric service delete method to validate
expected_db_version against the stored entity version and reject mismatches
consistently with the task/taskset fakes. Add a delete_metric test covering a
stale expected database version and assert the operation is rejected without
deleting the metric.
🤖 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 `@plugins/nemo-evaluator/tests/api/service/test_metric_service.py`:
- Around line 89-99: Update the fake metric service delete method to validate
expected_db_version against the stored entity version and reject mismatches
consistently with the task/taskset fakes. Add a delete_metric test covering a
stale expected database version and assert the operation is rejected without
deleting the metric.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6a628907-3d65-4512-8475-59a65206dd1f

📥 Commits

Reviewing files that changed from the base of the PR and between bb95212 and ad0ce55.

⛔ Files ignored due to path filters (14)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/entities/entities.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/entities/entity_delete_entity_by_name_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/guardrail/guardrail_config.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model_delete_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_step.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_task.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/inference/test_virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_entities.py is excluded by !sdk/**
📒 Files selected for processing (79)
  • docs/cli/reference.mdx
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/inference/virtual_models.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/entities.py
  • packages/nmp_common/tests/entities/test_client.py
  • plugins/example-plugin/src/nemo_example_plugin/middleware_service.py
  • plugins/example-plugin/src/nemo_example_plugin/service.py
  • plugins/example-plugin/tests/test_service.py
  • plugins/nemo-agents/openapi/openapi.yaml
  • plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
  • plugins/nemo-agents/tests/unit/test_agents_api.py
  • plugins/nemo-agents/tests/unit/test_runner_deployments.py
  • plugins/nemo-auditor/openapi/openapi.yaml
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/configs.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/targets.py
  • plugins/nemo-auditor/tests/test_api_configs.py
  • plugins/nemo-auditor/tests/test_api_targets.py
  • plugins/nemo-deployments/openapi/openapi.yaml
  • plugins/nemo-deployments/src/nemo_deployments_plugin/api/v2/deployment_configs.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/volume_reconciler.py
  • plugins/nemo-deployments/tests/unit/test_api_deployment_configs.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/metric_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/result_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/results.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py
  • plugins/nemo-evaluator/tests/api/service/test_metric_service.py
  • plugins/nemo-evaluator/tests/api/service/test_result_service.py
  • plugins/nemo-evaluator/tests/api/service/test_task_service.py
  • plugins/nemo-evaluator/tests/api/service/test_taskset_service.py
  • plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_results_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py
  • plugins/nemo-insights/src/nemo_insights_plugin/service.py
  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py
  • services/core/entities/src/nmp/core/entities/app/repository/entity.py
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/entity.py
  • services/core/entities/tests/repository/test_entity_delete_versioning.py
  • services/core/files/src/nmp/core/files/app/file_lock.py
  • services/core/files/tests/test_file_lock.py
  • services/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/virtual_models.py
  • services/core/inference-gateway/tests/unit/test_virtual_models_router.py
  • services/core/models/src/nmp/core/models/api/service/adapter_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_config_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_service.py
  • services/core/models/src/nmp/core/models/api/service/model_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_provider_service.py
  • services/core/models/src/nmp/core/models/api/service/prompt_service.py
  • services/core/models/src/nmp/core/models/api/v2/adapters.py
  • services/core/models/src/nmp/core/models/api/v2/deployment_configs.py
  • services/core/models/src/nmp/core/models/api/v2/deployments.py
  • services/core/models/src/nmp/core/models/api/v2/models.py
  • services/core/models/src/nmp/core/models/api/v2/prompts.py
  • services/core/models/src/nmp/core/models/api/v2/providers.py
  • services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
  • services/core/models/src/nmp/core/models/controllers/provider_reconciler.py
  • services/core/models/tests/unit/api/test_deployment_configs_api.py
  • services/core/models/tests/unit/api/test_deployments_api.py
  • services/core/models/tests/unit/api/test_models_api.py
  • services/core/models/tests/unit/api/test_prompts_api.py
  • services/core/models/tests/unit/api/test_providers_api.py
  • services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
  • services/core/models/tests/unit/controllers/test_provider_reconciler.py
  • services/core/models/tests/unit/test_model_deployment_config_service_unit.py
  • services/core/models/tests/unit/test_model_deployment_service_unit.py
  • services/core/models/tests/unit/test_model_entity_service_unit.py
  • services/core/models/tests/unit/test_model_provider_service_unit.py
  • services/core/models/tests/unit/test_prompt_service_unit.py
  • services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py
  • services/hello-world/src/nmp/hello_world/api/v1/messages/endpoints.py
🚧 Files skipped from review as they are similar to previous changes (59)
  • services/core/models/src/nmp/core/models/api/service/model_deployment_config_service.py
  • services/core/entities/tests/repository/test_entity_delete_versioning.py
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/inference/virtual_models.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
  • services/core/models/src/nmp/core/models/api/v2/providers.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_service.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
  • plugins/nemo-agents/openapi/openapi.yaml
  • services/core/inference-gateway/tests/unit/test_virtual_models_router.py
  • plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
  • services/core/models/tests/unit/api/test_deployment_configs_api.py
  • plugins/nemo-agents/tests/unit/test_runner_deployments.py
  • docs/cli/reference.mdx
  • services/core/models/src/nmp/core/models/controllers/provider_reconciler.py
  • services/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/virtual_models.py
  • services/core/models/tests/unit/test_prompt_service_unit.py
  • services/core/models/tests/unit/test_model_deployment_service_unit.py
  • services/core/models/src/nmp/core/models/api/service/model_entity_service.py
  • services/core/entities/src/nmp/core/entities/app/repository/entity.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/volume_reconciler.py
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/entity.py
  • services/core/models/src/nmp/core/models/api/v2/deployments.py
  • plugins/nemo-deployments/tests/unit/test_api_deployment_configs.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py
  • plugins/example-plugin/src/nemo_example_plugin/service.py
  • services/core/models/src/nmp/core/models/api/v2/prompts.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py
  • services/core/models/src/nmp/core/models/api/v2/models.py
  • services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py
  • plugins/nemo-deployments/openapi/openapi.yaml
  • services/core/models/tests/unit/controllers/test_provider_reconciler.py
  • services/core/models/src/nmp/core/models/api/v2/deployment_configs.py
  • services/core/models/tests/unit/test_model_entity_service_unit.py
  • plugins/example-plugin/src/nemo_example_plugin/middleware_service.py
  • services/hello-world/src/nmp/hello_world/api/v1/messages/endpoints.py
  • services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
  • services/core/models/src/nmp/core/models/api/service/prompt_service.py
  • services/core/models/tests/unit/api/test_models_api.py
  • services/core/models/tests/unit/test_model_deployment_config_service_unit.py
  • packages/nmp_common/tests/entities/test_client.py
  • services/core/models/src/nmp/core/models/api/service/model_provider_service.py
  • services/core/models/tests/unit/test_model_provider_service_unit.py
  • services/core/models/tests/unit/api/test_providers_api.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/results.py
  • services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
  • services/core/models/tests/unit/api/test_deployments_api.py
  • services/core/models/src/nmp/core/models/api/service/adapter_entity_service.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/configs.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/api/v2/deployment_configs.py
  • plugins/nemo-agents/tests/unit/test_agents_api.py
  • plugins/nemo-auditor/tests/test_api_configs.py
  • plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py
  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py
  • plugins/example-plugin/tests/test_service.py
  • services/core/files/tests/test_file_lock.py
  • services/core/files/src/nmp/core/files/app/file_lock.py
  • openapi/openapi.yaml
  • openapi/ga/openapi.yaml

@ironcommit
ironcommit force-pushed the fix-virtual-model-delete-toctou/rsadler branch from ad0ce55 to d1ff162 Compare July 28, 2026 20:26

@coderabbitai coderabbitai 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.

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)
web/packages/studio/src/mocks/handlers/guardrails.ts (1)

75-82: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Advance db_version on PATCH.

Line 81 changes the resource but leaves its version unchanged, so tests cannot observe stale optimistic operations after an update. Increment the stored version with each successful PATCH.

🤖 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 `@web/packages/studio/src/mocks/handlers/guardrails.ts` around lines 75 - 82,
Update the PATCH handler for guardrail configs to increment the matched config’s
stored db_version after applying the request body. Ensure every successful
update through this handler advances the version while preserving the existing
404 behavior and response.
🤖 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.

Inline comments:
In
`@services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/entity.py`:
- Around line 390-396: In the delete/commit flow, call await sess.rollback() in
the StaleDataError handler before raising EntityVersionConflictError, ensuring
the shared AsyncSession is usable for subsequent operations while preserving the
existing exception translation.

In `@services/hello-world/src/nmp/hello_world/api/v1/messages/endpoints.py`:
- Around line 128-130: Update the exception handling around the delete operation
in the endpoint to catch EntityNotFoundError before the generic Exception
handler and raise an HTTPException with status code 404. Preserve the existing
EntityConflictError mapping to 409 and generic error handling.

---

Outside diff comments:
In `@web/packages/studio/src/mocks/handlers/guardrails.ts`:
- Around line 75-82: Update the PATCH handler for guardrail configs to increment
the matched config’s stored db_version after applying the request body. Ensure
every successful update through this handler advances the version while
preserving the existing 404 behavior and response.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c0523f78-6cd6-494a-8000-71572d0b48fa

📥 Commits

Reviewing files that changed from the base of the PR and between ad0ce55 and d1ff162.

⛔ Files ignored due to path filters (14)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/entities/entities.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/entities/entity_delete_entity_by_name_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/guardrail/guardrail_config.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model_delete_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_step.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_task.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/inference/test_virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_entities.py is excluded by !sdk/**
📒 Files selected for processing (81)
  • docs/cli/reference.mdx
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/inference/virtual_models.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/entities.py
  • packages/nmp_common/tests/entities/test_client.py
  • plugins/example-plugin/src/nemo_example_plugin/middleware_service.py
  • plugins/example-plugin/src/nemo_example_plugin/service.py
  • plugins/example-plugin/tests/test_service.py
  • plugins/nemo-agents/openapi/openapi.yaml
  • plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
  • plugins/nemo-agents/tests/unit/test_agents_api.py
  • plugins/nemo-agents/tests/unit/test_runner_deployments.py
  • plugins/nemo-auditor/openapi/openapi.yaml
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/configs.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/targets.py
  • plugins/nemo-auditor/tests/test_api_configs.py
  • plugins/nemo-auditor/tests/test_api_targets.py
  • plugins/nemo-deployments/openapi/openapi.yaml
  • plugins/nemo-deployments/src/nemo_deployments_plugin/api/v2/deployment_configs.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/volume_reconciler.py
  • plugins/nemo-deployments/tests/unit/test_api_deployment_configs.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/metric_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/result_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/results.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py
  • plugins/nemo-evaluator/tests/api/service/test_metric_service.py
  • plugins/nemo-evaluator/tests/api/service/test_result_service.py
  • plugins/nemo-evaluator/tests/api/service/test_task_service.py
  • plugins/nemo-evaluator/tests/api/service/test_taskset_service.py
  • plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_results_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py
  • plugins/nemo-insights/src/nemo_insights_plugin/service.py
  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py
  • services/core/entities/src/nmp/core/entities/app/repository/entity.py
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/entity.py
  • services/core/entities/tests/repository/test_entity_delete_versioning.py
  • services/core/files/src/nmp/core/files/app/file_lock.py
  • services/core/files/tests/test_file_lock.py
  • services/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/virtual_models.py
  • services/core/inference-gateway/tests/unit/test_virtual_models_router.py
  • services/core/models/src/nmp/core/models/api/service/adapter_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_config_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_service.py
  • services/core/models/src/nmp/core/models/api/service/model_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_provider_service.py
  • services/core/models/src/nmp/core/models/api/service/prompt_service.py
  • services/core/models/src/nmp/core/models/api/v2/adapters.py
  • services/core/models/src/nmp/core/models/api/v2/deployment_configs.py
  • services/core/models/src/nmp/core/models/api/v2/deployments.py
  • services/core/models/src/nmp/core/models/api/v2/models.py
  • services/core/models/src/nmp/core/models/api/v2/prompts.py
  • services/core/models/src/nmp/core/models/api/v2/providers.py
  • services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
  • services/core/models/src/nmp/core/models/controllers/provider_reconciler.py
  • services/core/models/tests/unit/api/test_deployment_configs_api.py
  • services/core/models/tests/unit/api/test_deployments_api.py
  • services/core/models/tests/unit/api/test_models_api.py
  • services/core/models/tests/unit/api/test_prompts_api.py
  • services/core/models/tests/unit/api/test_providers_api.py
  • services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
  • services/core/models/tests/unit/controllers/test_provider_reconciler.py
  • services/core/models/tests/unit/test_model_deployment_config_service_unit.py
  • services/core/models/tests/unit/test_model_deployment_service_unit.py
  • services/core/models/tests/unit/test_model_entity_service_unit.py
  • services/core/models/tests/unit/test_model_provider_service_unit.py
  • services/core/models/tests/unit/test_prompt_service_unit.py
  • services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py
  • services/hello-world/src/nmp/hello_world/api/v1/messages/endpoints.py
  • web/packages/studio/src/mocks/handlers/guardrails.ts
  • web/packages/studio/src/routes/VirtualModelsListRoute/VirtualModelDetailsSidePanel/index.test.tsx
🚧 Files skipped from review as they are similar to previous changes (73)
  • plugins/example-plugin/src/nemo_example_plugin/service.py
  • services/core/models/tests/unit/test_model_deployment_config_service_unit.py
  • services/core/entities/tests/repository/test_entity_delete_versioning.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_service.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py
  • services/core/models/src/nmp/core/models/api/service/model_entity_service.py
  • services/core/models/src/nmp/core/models/api/v2/providers.py
  • plugins/nemo-deployments/openapi/openapi.yaml
  • services/core/models/src/nmp/core/models/api/v2/adapters.py
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/inference/virtual_models.py
  • plugins/nemo-agents/openapi/openapi.yaml
  • services/core/models/tests/unit/test_prompt_service_unit.py
  • plugins/nemo-auditor/openapi/openapi.yaml
  • services/core/models/src/nmp/core/models/api/service/adapter_entity_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py
  • services/core/models/tests/unit/test_model_deployment_service_unit.py
  • services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
  • services/core/inference-gateway/tests/unit/test_virtual_models_router.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py
  • services/core/models/src/nmp/core/models/api/v2/deployments.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/results.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py
  • plugins/nemo-agents/tests/unit/test_runner_deployments.py
  • plugins/nemo-insights/src/nemo_insights_plugin/service.py
  • plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
  • services/core/models/src/nmp/core/models/api/service/model_provider_service.py
  • services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py
  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py
  • services/core/models/src/nmp/core/models/api/v2/prompts.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_config_service.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/targets.py
  • docs/cli/reference.mdx
  • packages/nmp_common/tests/entities/test_client.py
  • services/core/models/tests/unit/api/test_deployments_api.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/api/v2/deployment_configs.py
  • services/core/models/src/nmp/core/models/controllers/provider_reconciler.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/volume_reconciler.py
  • services/core/models/src/nmp/core/models/api/v2/deployment_configs.py
  • services/core/models/src/nmp/core/models/api/v2/models.py
  • services/core/models/src/nmp/core/models/api/service/prompt_service.py
  • services/core/files/tests/test_file_lock.py
  • services/core/models/tests/unit/api/test_models_api.py
  • plugins/nemo-deployments/tests/unit/test_api_deployment_configs.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/metric_service.py
  • plugins/nemo-auditor/tests/test_api_targets.py
  • services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
  • plugins/nemo-agents/tests/unit/test_agents_api.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py
  • services/core/models/tests/unit/api/test_deployment_configs_api.py
  • services/core/files/src/nmp/core/files/app/file_lock.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py
  • services/core/models/tests/unit/api/test_prompts_api.py
  • services/core/models/tests/unit/test_model_provider_service_unit.py
  • plugins/nemo-auditor/tests/test_api_configs.py
  • services/core/models/tests/unit/test_model_entity_service_unit.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/result_service.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
  • plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py
  • services/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/virtual_models.py
  • services/core/models/tests/unit/api/test_providers_api.py
  • openapi/ga/individual/platform.openapi.yaml
  • plugins/example-plugin/tests/test_service.py
  • plugins/nemo-evaluator/tests/api/service/test_result_service.py
  • plugins/nemo-evaluator/tests/api/service/test_metric_service.py
  • services/core/models/tests/unit/controllers/test_provider_reconciler.py
  • plugins/nemo-evaluator/tests/api/service/test_task_service.py
  • openapi/ga/openapi.yaml
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/entities.py
  • openapi/openapi.yaml
  • plugins/nemo-evaluator/tests/api/service/test_taskset_service.py

@ironcommit
ironcommit force-pushed the fix-virtual-model-delete-toctou/rsadler branch 3 times, most recently from c02ec52 to cc8c7cb Compare July 28, 2026 22:02

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py`:
- Around line 200-208: Update the guarded delete block in the endpoint to also
catch EntityNotFoundError, mapping it to the same HTTP 404 response used when
the initial lookup cannot find the configuration. Preserve the existing
EntityConflictError handling and 409 response for concurrent modification
conflicts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 26d780d8-0ead-40bd-a005-86c40212ad58

📥 Commits

Reviewing files that changed from the base of the PR and between 6558451 and cc8c7cb.

⛔ Files ignored due to path filters (14)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/entities/entities.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/inference/virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/entities/entity_delete_entity_by_name_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/guardrail/guardrail_config.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/inference/virtual_model_delete_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_step.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/jobs/platform_job_task.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/inference/test_virtual_models.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_entities.py is excluded by !sdk/**
📒 Files selected for processing (91)
  • docs/cli/reference.mdx
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/inference/virtual_models.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/entities.py
  • packages/nmp_common/tests/entities/test_client.py
  • packages/nmp_testing/src/nmp/testing/utils.py
  • plugins/example-plugin/src/nemo_example_plugin/middleware_service.py
  • plugins/example-plugin/src/nemo_example_plugin/service.py
  • plugins/example-plugin/tests/test_service.py
  • plugins/nemo-agents/openapi/openapi.yaml
  • plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
  • plugins/nemo-agents/tests/unit/test_agents_api.py
  • plugins/nemo-agents/tests/unit/test_runner_deployments.py
  • plugins/nemo-auditor/openapi/openapi.yaml
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/configs.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/targets.py
  • plugins/nemo-auditor/tests/test_api_configs.py
  • plugins/nemo-auditor/tests/test_api_targets.py
  • plugins/nemo-deployments/openapi/openapi.yaml
  • plugins/nemo-deployments/src/nemo_deployments_plugin/api/v2/deployment_configs.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/volume_reconciler.py
  • plugins/nemo-deployments/tests/unit/test_api_deployment_configs.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/metric_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/result_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/results.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py
  • plugins/nemo-evaluator/tests/api/service/test_metric_service.py
  • plugins/nemo-evaluator/tests/api/service/test_result_service.py
  • plugins/nemo-evaluator/tests/api/service/test_task_service.py
  • plugins/nemo-evaluator/tests/api/service/test_taskset_service.py
  • plugins/nemo-evaluator/tests/api/v2/test_metrics_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_results_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py
  • plugins/nemo-insights/src/nemo_insights_plugin/service.py
  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py
  • services/core/entities/src/nmp/core/entities/app/repository/entity.py
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/entity.py
  • services/core/entities/tests/repository/test_entity_delete_versioning.py
  • services/core/files/src/nmp/core/files/app/file_lock.py
  • services/core/files/tests/test_file_lock.py
  • services/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/virtual_models.py
  • services/core/inference-gateway/tests/integration/test_inference.py
  • services/core/inference-gateway/tests/integration/test_middleware_pipeline.py
  • services/core/inference-gateway/tests/unit/conftest.py
  • services/core/inference-gateway/tests/unit/test_middleware_registry.py
  • services/core/inference-gateway/tests/unit/test_model_router.py
  • services/core/inference-gateway/tests/unit/test_openai_router.py
  • services/core/inference-gateway/tests/unit/test_proxy.py
  • services/core/inference-gateway/tests/unit/test_virtual_model_cache.py
  • services/core/inference-gateway/tests/unit/test_virtual_models_router.py
  • services/core/models/src/nmp/core/models/api/service/adapter_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_config_service.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_service.py
  • services/core/models/src/nmp/core/models/api/service/model_entity_service.py
  • services/core/models/src/nmp/core/models/api/service/model_provider_service.py
  • services/core/models/src/nmp/core/models/api/service/prompt_service.py
  • services/core/models/src/nmp/core/models/api/v2/adapters.py
  • services/core/models/src/nmp/core/models/api/v2/deployment_configs.py
  • services/core/models/src/nmp/core/models/api/v2/deployments.py
  • services/core/models/src/nmp/core/models/api/v2/models.py
  • services/core/models/src/nmp/core/models/api/v2/prompts.py
  • services/core/models/src/nmp/core/models/api/v2/providers.py
  • services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
  • services/core/models/src/nmp/core/models/controllers/provider_reconciler.py
  • services/core/models/tests/unit/api/test_deployment_configs_api.py
  • services/core/models/tests/unit/api/test_deployments_api.py
  • services/core/models/tests/unit/api/test_models_api.py
  • services/core/models/tests/unit/api/test_prompts_api.py
  • services/core/models/tests/unit/api/test_providers_api.py
  • services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
  • services/core/models/tests/unit/controllers/test_provider_reconciler.py
  • services/core/models/tests/unit/test_model_deployment_config_service_unit.py
  • services/core/models/tests/unit/test_model_deployment_service_unit.py
  • services/core/models/tests/unit/test_model_entity_service_unit.py
  • services/core/models/tests/unit/test_model_provider_service_unit.py
  • services/core/models/tests/unit/test_prompt_service_unit.py
  • services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py
  • services/hello-world/src/nmp/hello_world/api/v1/messages/endpoints.py
  • services/hello-world/tests/unit/test_messages_endpoints.py
  • web/packages/studio/src/mocks/handlers/guardrails.ts
  • web/packages/studio/src/routes/VirtualModelsListRoute/VirtualModelDetailsSidePanel/index.test.tsx
🚧 Files skipped from review as they are similar to previous changes (74)
  • web/packages/studio/src/mocks/handlers/guardrails.ts
  • plugins/nemo-insights/src/nemo_insights_plugin/service.py
  • web/packages/studio/src/routes/VirtualModelsListRoute/VirtualModelDetailsSidePanel/index.test.tsx
  • services/core/models/src/nmp/core/models/api/v2/adapters.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_config_service.py
  • services/core/models/src/nmp/core/models/api/v2/prompts.py
  • services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
  • services/core/entities/src/nmp/core/entities/app/repository/entity.py
  • docs/cli/reference.mdx
  • services/hello-world/src/nmp/hello_world/api/v1/messages/endpoints.py
  • services/core/models/src/nmp/core/models/api/service/model_deployment_service.py
  • services/core/models/src/nmp/core/models/api/service/model_provider_service.py
  • services/core/models/tests/unit/api/test_models_api.py
  • services/core/models/tests/unit/test_prompt_service_unit.py
  • services/core/models/src/nmp/core/models/api/service/prompt_service.py
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/inference/virtual_models.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/targets.py
  • services/core/models/src/nmp/core/models/api/v2/models.py
  • services/core/models/tests/unit/api/test_prompts_api.py
  • plugins/nemo-agents/src/nemo_agents_plugin/api/v2/agents.py
  • services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
  • services/core/models/tests/unit/api/test_providers_api.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py
  • services/core/models/tests/unit/test_model_deployment_config_service_unit.py
  • services/core/inference-gateway/tests/unit/test_virtual_models_router.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/api/v2/deployment_configs.py
  • plugins/nemo-agents/openapi/openapi.yaml
  • plugins/nemo-evaluator/tests/api/v2/test_tasks_routes.py
  • services/core/models/tests/unit/api/test_deployment_configs_api.py
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/entity.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasksets.py
  • plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/volume_reconciler.py
  • services/core/entities/src/nmp/core/entities/api/v2/entities/endpoints.py
  • services/core/models/src/nmp/core/models/controllers/provider_reconciler.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/taskset_service.py
  • plugins/nemo-evaluator/tests/api/service/test_task_service.py
  • services/core/models/tests/unit/test_model_provider_service_unit.py
  • plugins/nemo-auditor/src/nemo_auditor/api/v2/configs.py
  • plugins/nemo-agents/src/nemo_agents_plugin/runner/controller.py
  • services/core/models/src/nmp/core/models/api/service/adapter_entity_service.py
  • plugins/example-plugin/src/nemo_example_plugin/service.py
  • services/core/files/tests/test_file_lock.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py
  • plugins/example-plugin/src/nemo_example_plugin/middleware_service.py
  • services/core/models/tests/unit/test_model_entity_service_unit.py
  • plugins/nemo-auditor/openapi/openapi.yaml
  • plugins/nemo-agents/tests/unit/test_agents_api.py
  • services/core/models/src/nmp/core/models/api/service/model_entity_service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/tasks.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/result_service.py
  • plugins/nemo-auditor/tests/test_api_targets.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/v2/results.py
  • plugins/nemo-evaluator/tests/api/service/test_result_service.py
  • plugins/example-plugin/tests/test_service.py
  • plugins/nemo-evaluator/tests/api/v2/test_tasksets_routes.py
  • plugins/nemo-agents/tests/unit/test_runner_deployments.py
  • services/core/models/src/nmp/core/models/api/v2/deployments.py
  • plugins/nemo-deployments/openapi/openapi.yaml
  • plugins/nemo-evaluator/tests/api/service/test_taskset_service.py
  • services/core/models/src/nmp/core/models/api/v2/deployment_configs.py
  • services/core/inference-gateway/src/nmp/core/inference_gateway/api/v2/virtual_models.py
  • services/core/models/src/nmp/core/models/api/v2/providers.py
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/task_service.py
  • services/core/models/tests/unit/api/test_deployments_api.py
  • plugins/nemo-evaluator/tests/api/service/test_metric_service.py
  • openapi/ga/openapi.yaml
  • plugins/nemo-evaluator/src/nemo_evaluator/api/service/metric_service.py
  • services/core/models/tests/unit/test_model_deployment_service_unit.py
  • packages/nmp_common/tests/entities/test_client.py
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/openapi.yaml
  • services/core/files/src/nmp/core/files/app/file_lock.py
  • plugins/nemo-evaluator/tests/api/v2/test_results_routes.py

Comment thread services/guardrails/src/nmp/guardrails/api/v2/configs/endpoints.py
@ironcommit
ironcommit force-pushed the fix-virtual-model-delete-toctou/rsadler branch 4 times, most recently from eff3fdd to 74e60cc Compare July 28, 2026 23:42
Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
@ironcommit
ironcommit force-pushed the fix-virtual-model-delete-toctou/rsadler branch from 74e60cc to 41f9fe3 Compare July 29, 2026 01:58

@mikeknep mikeknep 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.

Nice, LGTM

@ironcommit
ironcommit added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit d77654d Jul 29, 2026
64 checks passed
@ironcommit
ironcommit deleted the fix-virtual-model-delete-toctou/rsadler branch July 29, 2026 16:52
maxdubrinsky added a commit that referenced this pull request Jul 29, 2026
Main dropped the legacy docker and k8s-nim-operator backends (#705) and
replaced the reconcilers' per-entity Model Entity reads and writes with
ModelEntityCache (#953, #951). The branch's changes to the deleted backends
were import rewrites, so they go with the files. Everywhere the cache
supersedes a direct read/write, main's semantics win and the calls it makes
are routed through the typed client.

ModelEntityCache arrived on main built on the umbrella SDK, which left
models_controller holding a dangling self._models_sdk after the merge. It is
migrated to AsyncModelsClient here, since leaving it on the SDK would make
the branch's premise only half true.

_load_virtual_models still reached for self._models_sdk. Its bare
except Exception reported the AttributeError as a VirtualModel listing
failure and skipped orphan cleanup, so the tests stayed green while the
cleanup silently never ran.

VirtualModel work stays on the umbrella SDK: it is an inference-gateway
resource, not a Models one. The SDK exception types are aliased to say so.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants