ci: fix workflow required-check names for feature/koosh-migrate-1233-feat-termux-support#492
Conversation
…migrate-1233-feat-termux-support
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis change consolidates SDK configuration types by introducing type aliases to external package definitions, adds a GitHub Actions workflow for automatic PR merging with label controls, and updates workflow configurations for build and path guarding processes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-merge.yml:
- Around line 20-24: The job defined as "enable-automerge" lacks a
human-readable name; add a top-level "name" field to that job (e.g., name:
"Enable Auto-merge") so it matches other workflows and can be selected as a
required status check; update the job block containing the if condition and
runs-on (the "enable-automerge" job) to include the new name field immediately
under the job key.
In @.github/workflows/pr-path-guard.yml:
- Line 25: The if condition in the GitHub Actions step duplicates the
branch-prefix negation clause — remove the redundant repeated check so the
condition reads: steps.changed-files.outputs.any_changed == 'true' &&
!(startsWith(github.head_ref, 'feature/koosh-migrate') ||
startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref,
'migrated/')); edit the single line containing the long if expression to remove
the second occurrence of "!(startsWith(github.head_ref, 'feature/koosh-migrate')
|| startsWith(github.head_ref, 'feature/migrate-') ||
startsWith(github.head_ref, 'migrated/'))" so the logic remains the same but
without the duplicate clause.
In `@pkg/llmproxy/access/reconcile.go`:
- Around line 89-98: The field-by-field copy into sdkCfg is redundant because
config.SDKConfig is a type alias for sdkconfig.SDKConfig; replace the manual
struct literal with a direct assignment from newCfg.SDKConfig to sdkCfg (e.g.,
sdkCfg := newCfg.SDKConfig) so you keep a local copy and avoid mutating the
original, and then call configaccess.Register(&sdkCfg); remove the unnecessary
sdkconfig.StreamingConfig conversion and the individual field copies.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/auto-merge.yml.github/workflows/pr-path-guard.yml.github/workflows/pr-test-build.ymlpkg/llmproxy/access/reconcile.gopkg/llmproxy/api/handlers/management/config_basic.gopkg/llmproxy/config/sdk_types.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (Go) (go)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: NEVER create a v2 file - refactor the original instead
NEVER create a new class if an existing one can be made generic
NEVER create custom implementations when an OSS library exists - search pkg.go.dev for existing libraries before writing code
Build generic building blocks (provider interface + registry) before application logic
Use chi for HTTP routing (NOT custom routers)
Use zerolog for logging (NOT fmt.Print)
Use viper for configuration (NOT manual env parsing)
Use go-playground/validator for validation (NOT manual if/else validation)
Use golang.org/x/time/rate for rate limiting (NOT custom limiters)
Use template strings for messages instead of hardcoded messages and config-driven logic instead of code-driven
Zero new lint suppressions without inline justification
All new code must pass: go fmt, go vet, golint
Maximum function length: 40 lines
No placeholder TODOs in committed code
Files:
pkg/llmproxy/config/sdk_types.gopkg/llmproxy/access/reconcile.gopkg/llmproxy/api/handlers/management/config_basic.go
🧠 Learnings (1)
📚 Learning: 2026-02-25T10:11:41.448Z
Learnt from: CR
Repo: KooshaPari/cliproxyapi-plusplus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-25T10:11:41.448Z
Learning: Applies to **/*.go : Build generic building blocks (provider interface + registry) before application logic
Applied to files:
pkg/llmproxy/api/handlers/management/config_basic.go
🧬 Code graph analysis (1)
pkg/llmproxy/api/handlers/management/config_basic.go (1)
pkg/llmproxy/config/sdk_types.go (1)
SDKConfig(7-7)
🔇 Additional comments (3)
.github/workflows/pr-test-build.yml (1)
11-11: LGTM!Explicitly naming the job ensures the required status check name is predictable and stable for branch protection rules.
pkg/llmproxy/config/sdk_types.go (1)
1-8: LGTM!Using type aliases to re-export SDK configuration types is a clean approach that prevents type divergence while maintaining backward compatibility for existing code that imports from
config.pkg/llmproxy/api/handlers/management/config_basic.go (1)
47-48: LGTM!The change correctly uses the type alias
config.SDKConfig, which resolves tosdkconfig.SDKConfigper the alias defined insdk_types.go. This maintains consistency across the codebase.
| enable-automerge: | ||
| if: | | ||
| (github.event_name != 'pull_request_review') || | ||
| (github.event.review.state == 'APPROVED') | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a job name for consistency.
Other workflows in this PR explicitly name their jobs for required status check alignment. If this job should appear in required checks, consider adding a name field.
♻️ Proposed fix
jobs:
enable-automerge:
+ name: enable-automerge
if: |
(github.event_name != 'pull_request_review') ||
(github.event.review.state == 'APPROVED')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| enable-automerge: | |
| if: | | |
| (github.event_name != 'pull_request_review') || | |
| (github.event.review.state == 'APPROVED') | |
| runs-on: ubuntu-latest | |
| enable-automerge: | |
| name: enable-automerge | |
| if: | | |
| (github.event_name != 'pull_request_review') || | |
| (github.event.review.state == 'APPROVED') | |
| runs-on: ubuntu-latest |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/auto-merge.yml around lines 20 - 24, The job defined as
"enable-automerge" lacks a human-readable name; add a top-level "name" field to
that job (e.g., name: "Enable Auto-merge") so it matches other workflows and can
be selected as a required status check; update the job block containing the if
condition and runs-on (the "enable-automerge" job) to include the new name field
immediately under the job key.
| internal/translator/** | ||
| - name: Fail when restricted paths change | ||
| if: steps.changed-files.outputs.any_changed == 'true' && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/')) | ||
| if: steps.changed-files.outputs.any_changed == 'true' && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/')) && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/')) |
There was a problem hiding this comment.
Redundant duplicate condition detected.
The if condition contains the same branch-prefix check twice, which is logically redundant. This appears to be a copy-paste error.
🔧 Proposed fix to remove duplicate condition
- if: steps.changed-files.outputs.any_changed == 'true' && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/')) && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/'))
+ if: steps.changed-files.outputs.any_changed == 'true' && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/'))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if: steps.changed-files.outputs.any_changed == 'true' && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/')) && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/')) | |
| if: steps.changed-files.outputs.any_changed == 'true' && !(startsWith(github.head_ref, 'feature/koosh-migrate') || startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref, 'migrated/')) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-path-guard.yml at line 25, The if condition in the
GitHub Actions step duplicates the branch-prefix negation clause — remove the
redundant repeated check so the condition reads:
steps.changed-files.outputs.any_changed == 'true' &&
!(startsWith(github.head_ref, 'feature/koosh-migrate') ||
startsWith(github.head_ref, 'feature/migrate-') || startsWith(github.head_ref,
'migrated/')); edit the single line containing the long if expression to remove
the second occurrence of "!(startsWith(github.head_ref, 'feature/koosh-migrate')
|| startsWith(github.head_ref, 'feature/migrate-') ||
startsWith(github.head_ref, 'migrated/'))" so the logic remains the same but
without the duplicate clause.
| sdkCfg := sdkconfig.SDKConfig{ | ||
| ProxyURL: newCfg.SDKConfig.ProxyURL, | ||
| ForceModelPrefix: newCfg.SDKConfig.ForceModelPrefix, | ||
| RequestLog: newCfg.SDKConfig.RequestLog, | ||
| APIKeys: newCfg.SDKConfig.APIKeys, | ||
| PassthroughHeaders: newCfg.SDKConfig.PassthroughHeaders, | ||
| Streaming: sdkconfig.StreamingConfig(newCfg.SDKConfig.Streaming), | ||
| NonStreamKeepAliveInterval: newCfg.SDKConfig.NonStreamKeepAliveInterval, | ||
| } | ||
| configaccess.Register(&sdkCfg) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Field-by-field copy is redundant due to type aliasing.
Since config.SDKConfig is now a type alias for sdkconfig.SDKConfig (defined in sdk_types.go), these types are identical. The explicit field-by-field copy and StreamingConfig conversion are unnecessary.
♻️ Proposed simplification
existing := manager.Providers()
- sdkCfg := sdkconfig.SDKConfig{
- ProxyURL: newCfg.SDKConfig.ProxyURL,
- ForceModelPrefix: newCfg.SDKConfig.ForceModelPrefix,
- RequestLog: newCfg.SDKConfig.RequestLog,
- APIKeys: newCfg.SDKConfig.APIKeys,
- PassthroughHeaders: newCfg.SDKConfig.PassthroughHeaders,
- Streaming: sdkconfig.StreamingConfig(newCfg.SDKConfig.Streaming),
- NonStreamKeepAliveInterval: newCfg.SDKConfig.NonStreamKeepAliveInterval,
- }
- configaccess.Register(&sdkCfg)
+ configaccess.Register(&newCfg.SDKConfig)If you need a local copy to avoid mutating the original config, a simple assignment suffices:
sdkCfg := newCfg.SDKConfig
configaccess.Register(&sdkCfg)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/llmproxy/access/reconcile.go` around lines 89 - 98, The field-by-field
copy into sdkCfg is redundant because config.SDKConfig is a type alias for
sdkconfig.SDKConfig; replace the manual struct literal with a direct assignment
from newCfg.SDKConfig to sdkCfg (e.g., sdkCfg := newCfg.SDKConfig) so you keep a
local copy and avoid mutating the original, and then call
configaccess.Register(&sdkCfg); remove the unnecessary sdkconfig.StreamingConfig
conversion and the individual field copies.
KooshaPari
added
HELIOS-CODEX
|
[HELIOS-CODEX] Retro-versioned and grouped.
This PR is aligned for stacked/layered merge in the HELIOS-CODEX run and should merge after any earlier wave dependency and before any later-wave dependency. |
|
[HELIOS-CODEX] Retro-versioned and grouped.\n\n- version stamp: HELIOS-CODEX/v6.8.87-0-codex0.101.0\n- effective semantic version: v6.8.87-0+codex0.101.0\n- package: pkg:he:termux\n- layer: HELIOS-CODEX-L0\n- stack lane: Wave A (foundation)\n- branch scope: feature/koosh-migrate-1233-feat-termux-support\n- change package: termux support migration\n\nThis PR is aligned for stacked/layered merge in the HELIOS-CODEX run and should merge after any earlier wave dependency and before any later-wave dependency. |
|
[HELIOS-CODEX] Versioning rule corrected.\n\nUse this pattern from now on: <project_version>-<codex_minor>.<codex_patch>.\n\n- project version: 0.1.3\n- codex version: 0.123.1\n- corrected composite version: 0.1.3-${codex_version#0.} |
HELIOS-CODEX versioning correctionUse the corrected composite rule:
Please keep prior HELIOS notes but normalize package/version fields to this format in labels, release notes, and future comments. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
KooshaPari
merged commit d6ea62e
into
feature/koosh-migrate-1233-feat-termux-support
KooshaPari
deleted the
ci/fix-feature-koosh-migrate-1233-feat-termux-support
branch
1 participant
CI/workflow fixes for required-checks and path-guard/build naming.
Summary by CodeRabbit
New Features
Chores
Refactor