feat(executor): fix payloadModelRulesMatch for unconditional rules#927
feat(executor): fix payloadModelRulesMatch for unconditional rules#927KooshaPari merged 3 commits intomainfrom
Conversation
…r packages This commit removes redundant split files that were created during the refactor split, consolidating each package back to a single implementation source: management package: - Removed auth_file_mgmt.go, auth_gemini.go (duplicates of auth_files.go) - Removed auth_helpers.go, auth_file_crud.go, auth_file_patch.go (duplicates) - Removed auth_kiro.go, auth_status.go (duplicates) - Removed auth_anthropic.go, auth_antigravity.go, auth_codex.go - Removed auth_github.go, auth_iflow.go, auth_kilo.go, auth_kimi.go, auth_qwen.go executor package: - Removed kiro_streaming_event_parser.go, kiro_streaming_transform.go - Removed kiro_streaming_websearch.go, kiro_streaming_fallback.go - Removed kiro_auth.go, kiro_transform.go, kiro_streaming_init.go config package: - Removed the entire split config module (config_io.go, config_persistence.go, config_providers.go, config_types.go, config_validation.go) The canonical implementations in the monolith files remain unchanged. Additional changes: - Updated import paths from sdk/auth to sdk/cliproxy/auth across affected files - Added Minimax thinking provider package
- Fix payloadModelRulesMatch to treat empty Name as unconditional rule - Add regression tests for split-count coverage (conditional vs unconditional) - Tests cover protocol matching, alias targeting, and filter rules - Include TestPayloadModelRulesMatch, TestPayloadModelCandidates - Include TestApplyPayloadConfigWithRoot_* (6 test functions, 30+ cases)
- Add path-escape validation to resolveAuthPath - Ensure all file paths stay within configured base directory - Normalize paths with filepath.Clean before validation - Fixes CodeQL go/path-injection alerts
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Caution Review failedThe pull request is closed. Note
|
| Cohort / File(s) | Summary |
|---|---|
Documentation cleanup CODE_REFACTOR_SUMMARY.md, REFACTOR_COMPLETE_SUMMARY.md, REFACTOR_SESSION_FINAL_SUMMARY.md, REFACTOR_SESSION_SUMMARY.md, SDK_INTEGRATION_STATUS.md |
Removed session documentation and status tracking files totaling ~560 lines. |
Config package restructuring pkg/llmproxy/config/config_io.go, config_persistence.go, config_providers.go, config_types.go, config_validation.go |
Deleted entire configuration infrastructure including YAML loading/persistence (~295, ~670, ~37, ~620, ~460 lines respectively), sanitization routines, and type definitions. Environment override logic, comment preservation, and validation removed. |
SDK config update pkg/llmproxy/config/sdk_config.go |
Modified to define new SDKConfig struct with ProxyURL field; removed internalconfig import dependency. |
Import migration: Config pkg/llmproxy/api/.../*, pkg/llmproxy/auth/.../*, pkg/llmproxy/executor/.../*, pkg/llmproxy/cmd/.../*, pkg/llmproxy/watcher/.../*, pkg/llmproxy/util/.../*, ~50+ files |
Updated all imports from internal/config to pkg/llmproxy/config, affecting Config type references across handlers, auth modules, executors, and utilities. |
Import migration: Translator & Auth pkg/llmproxy/translator/.../*, pkg/llmproxy/auth/.../*, pkg/llmproxy/watcher/events.go |
Relocated translator and auth imports from internal/ to pkg/llmproxy/ namespace including Gemini common, Codex, and Kiro auth modules. |
Payload & stream handling pkg/llmproxy/executor/payload_helpers.go, pkg/llmproxy/executor/payload_helpers_test.go, pkg/llmproxy/translator/openai/claude/openai_claude_response.go |
Enhanced empty models/rules matching logic; refactored OpenAI→Claude response streaming with upfront stream detection and flattened control flow; added 367 lines of payload helper unit tests. |
Thinking providers pkg/llmproxy/thinking/provider/minimax/minimax.go, pkg/llmproxy/thinking/apply.go, pkg/llmproxy/thinking/provider/iflow/apply.go, pkg/llmproxy/thinking/strip.go |
Added minimax provider registration; extended iFlow config parsing for reasoning effort with normalized modes (None/Auto/Level); updated MiniMax field deletion and iFlow JSON path pruning. |
SDK integration sdk/config/config.go, sdk/auth/filestore.go, pkg/llmproxy/util/proxy.go |
Switched type aliases from pkgconfig.* to llmproxyconfig.*; refactored auth path resolution to validate base-directory containment; expanded proxy URL handling with sanitization and nil client allocation. |
Test & build go.mod, test/amp_management_test.go, test/builtin_tools_translation_test.go, test/thinking_conversion_test.go |
Reformatted go.mod indentation; updated test imports to pkg/llmproxy packages; added minimax provider to thinking conversion tests. |
Minor updates pkg/llmproxy/auth/vertex/vertex_credentials.go, pkg/llmproxy/executor/thinking_providers.go, pkg/llmproxy/interfaces/error_message.go, pkg/llmproxy/registry/model_registry.go |
Added Vertex auth directory constant; added minimax blank import; updated misc/interfaces package paths. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- backup:\ pre-wave\ full\ dirty\ snapshot\ before\ fresh-main\ worktree\ execution #410: Conflicts directly with config type deletions and related
IsResponsesCompactEnabledmethod removal. - Integrate phenotype-go-kit for auth token storage #866: Overlaps on auth token storage import changes in
pkg/llmproxy/auth/qwen/qwen_token.go. - codex/stability fix 1 #861: Shares thinking-effort and minimax provider logic modifications across apply/strip/iflow handling.
Suggested labels
HELIOS-CODEX, HELIOS-CODEX-L0, refactor, package-migration
Poem
🐰 From
internaldepths topkglight so bright,
Config's structure now dances in public sight,
Minimax whispers join the thinking choir,
Handlers and proxies climb the package spire!
Refactored and relocated with code-review care. ✨
✨ 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/pr-11-event-order-rebased
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 @coderabbitai help to get the list of available commands and usage tips.
Summary
Rebased version of PR #924 to resolve merge conflicts with main.
Files Changed
Supersedes PR #924 (rebased onto current main, conflicts resolved by keeping PR branch version).
Summary by CodeRabbit
New Features
Bug Fixes
Chores