Skip to content

removes vk model check for batch/containers/files requests#1471

Merged
akshaydeo merged 1 commit intomainfrom
01-29-removes_vk_model_check_for_batch_containers_files_requests
Jan 29, 2026
Merged

removes vk model check for batch/containers/files requests#1471
akshaydeo merged 1 commit intomainfrom
01-29-removes_vk_model_check_for_batch_containers_files_requests

Conversation

@akshaydeo
Copy link
Copy Markdown
Contributor

@akshaydeo akshaydeo commented Jan 29, 2026

Summary

Added model requirement checking for governance requests to prevent unnecessary model filtering for certain request types.

Changes

  • Added isModelRequired function to determine if a model check is needed based on request type
  • Modified model filtering logic to only check when the model is required
  • Identified request types that don't need model validation (batches, containers, files, etc.)
  • Cleaned up code by removing unnecessary blank lines

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (Next.js)
  • Docs

How to test

Test various request types to ensure proper governance filtering:

# Core/Transports
go test ./plugins/governance/...

# Test with different request types
# Model-required requests (chat, completion) should validate models
# Non-model requests (files, batches, containers) should skip model validation

Breaking changes

  • Yes
  • No

Related issues

Fixes issues with governance filtering blocking valid non-model requests

Security considerations

This change maintains proper governance controls while allowing appropriate access to non-model resources.

Checklist

  • I added/updated tests where appropriate
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 29, 2026

Caution

Review failed

The head commit changed during the review from 3af9808 to 2ef54c9.

📝 Walkthrough

Walkthrough

Governance request evaluation is refactored to conditionally validate models based on request type. A new helper method determines whether model allowlisting applies to specific request types, and model validation logic is updated accordingly. Comment and formatting adjustments made to improve code clarity.

Changes

Cohort / File(s) Summary
Governance Request Evaluation
plugins/governance/main.go, plugins/governance/resolver.go
Added conditional model validation logic via new isModelRequired helper method that skips model allowlisting checks for operations like MCPToolExecution and Batch/File/Container operations. Model filtering in EvaluateVirtualKeyRequest is now selective, while provider-level checking remains unchanged. Minor comment and formatting adjustments included.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Hop through governance with newfound grace,
Where models find their rightful place,
Some tasks need not their mark to bear,
Conditional wisdom floating through the air! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: removing model validation checks for specific request types (batches, containers, files).
Description check ✅ Passed The description covers all major required sections with appropriate detail: summary, changes, type of change, affected areas, testing instructions, breaking changes, and related issues.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 01-29-removes_vk_model_check_for_batch_containers_files_requests

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

Copy link
Copy Markdown
Contributor Author

akshaydeo commented Jan 29, 2026

@akshaydeo akshaydeo marked this pull request as ready for review January 29, 2026 12:35
@akshaydeo akshaydeo force-pushed the 01-29-removes_vk_model_check_for_batch_containers_files_requests branch from 2b49967 to 3af9808 Compare January 29, 2026 13:10
@akshaydeo akshaydeo force-pushed the 01-29-removes_vk_model_check_for_batch_containers_files_requests branch from 3af9808 to 2ef54c9 Compare January 29, 2026 13:21
@akshaydeo akshaydeo force-pushed the 01-29-fixes_race_condition_while_loading_mcp_servers_from_config_file branch from d46d034 to 8f6f420 Compare January 29, 2026 13:21
Copy link
Copy Markdown
Contributor Author

akshaydeo commented Jan 29, 2026

Merge activity

  • Jan 29, 1:33 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jan 29, 1:35 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo akshaydeo changed the base branch from 01-29-fixes_race_condition_while_loading_mcp_servers_from_config_file to graphite-base/1471 January 29, 2026 13:34
@akshaydeo akshaydeo changed the base branch from graphite-base/1471 to main January 29, 2026 13:34
@akshaydeo akshaydeo merged commit 8da2cae into main Jan 29, 2026
4 checks passed
@akshaydeo akshaydeo deleted the 01-29-removes_vk_model_check_for_batch_containers_files_requests branch January 29, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant