fix: resolve executor compile regressions#264
fix: resolve executor compile regressions#264KooshaPari wants to merge 421 commits intorouter-for-me:mainfrom
Conversation
# Conflicts: # docs/planning/reports/issue-wave-gh-next21-lane-6.md # pkg/llmproxy/api/modules/amp/routes_test.go
These fields are not supported by Codex Responses API and cause upstream errors. Fixes: router-for-me#1667
- Replace manual prefix/suffix checks with strings.TrimPrefix/TrimSuffix - Remove unused functions and variables in kiro_executor.go and request_logger.go - Move default case to the end of switch statement in kiro_executor.go - Fix function signatures and call sites in kiro_executor.go where arguments were immediately overwritten Co-authored-by: Cursor <cursoragent@cursor.com>
- Added unit tests for internal/auth packages (codex, copilot, gemini, kiro). - Added unit tests for internal/runtime/executor, specifically for Kiro executor. - Added unit tests for various internal translators. - Improved internal/api and sdk test coverage. - Fixed a race condition in Kiro social authentication callback server. - Updated .gitignore to exclude build artifacts and coverage files. - Cleaned up go.mod and go.sum with go mod tidy. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…rs and fix kiro_websearch functions This commit: - Updates cmd/codegen/main.go to support multiple templates and directories. - Embeds GeneratedConfig in internal/config/Config. - Restores SSE and message ID functions in internal/translator/kiro/claude/kiro_websearch.go. - Adds pkg/llmproxy/ directory structure for shared components. - Fixes missing strings import in generated config files. Co-authored-by: Cursor <cursoragent@cursor.com>
Renamed references to CLIProxyAPIPlus to cliproxyapi++ (display) and cliproxyapi-plusplus (technical). Updated GitHub remote and internal URLs. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…260223-v2 docs: replace marketing language with concise technical wording
…-20260223 merge: absorb heliosharness parity branch functional delta
Summary of ChangesHello @KooshaPari, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents a significant step in enhancing the project's architecture, developer experience, and operational robustness. It transitions core components to a library-first design, introduces a comprehensive documentation framework, and establishes robust CI/CD pipelines and quality gates. These changes aim to improve reusability, maintainability, and the overall reliability of the proxy, while also streamlining development and release processes. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully migrates core logic to a public pkg/llmproxy package, improving reusability and following a library-first architecture. However, it introduces several critical compilation errors in cmd/server/main.go due to undefined variables and functions in the newly added configuration path inspection block. Additionally, the linter configuration in .golangci.yml explicitly disables shadowing checks, which contradicts the PR's objective of resolving variable shadowing issues.
| func resolveDefaultConfigPath(wd string, isCloudDeploy bool) string { | ||
| fallback := filepath.Join(wd, "config.yaml") | ||
| candidates := make([]string, 0, 12) | ||
|
|
||
| addEnvCandidate := func(key string) { | ||
| value := strings.TrimSpace(os.Getenv(key)) | ||
| if value != "" { | ||
| candidates = append(candidates, value) | ||
| } | ||
| } | ||
| addEnvCandidate("CONFIG") | ||
| addEnvCandidate("CONFIG_PATH") | ||
| addEnvCandidate("CLIPROXY_CONFIG") | ||
| addEnvCandidate("CLIPROXY_CONFIG_PATH") | ||
|
|
||
| candidates = append(candidates, fallback) | ||
| // If config.yaml is mounted as a directory (common Docker mis-mount), | ||
| // prefer the nested config/config.yaml path before failing on the directory. | ||
| candidates = append(candidates, filepath.Join(wd, "config", "config.yaml")) | ||
| if isCloudDeploy { | ||
| candidates = append(candidates, | ||
| "/CLIProxyAPI/config.yaml", | ||
| "/CLIProxyAPI/config/config.yaml", | ||
| "/config/config.yaml", | ||
| "/app/config.yaml", | ||
| "/app/config/config.yaml", | ||
| ) | ||
| } | ||
|
|
||
| for _, candidate := range candidates { | ||
| if isReadableConfigFile(candidate) { | ||
| return candidate | ||
| } | ||
| } | ||
| return fallback | ||
| } |
There was a problem hiding this comment.
The function resolveDefaultConfigPathWithCandidates is called in main.go but is not defined here. It appears the implementation of this helper was omitted from the PR, which will lead to a compilation error.
| if showConfigPaths { | ||
| selected, candidates := resolveDefaultConfigPathWithCandidates(wd, isCloudDeploy) | ||
| fmt.Println("Config path candidates:") | ||
| if strings.TrimSpace(configPath) != "" { | ||
| fmt.Printf("* %s [from --config]\n", configPath) | ||
| } | ||
| printConfigCandidates(selected, candidates) | ||
| fmt.Printf("Selected: %s\n", selected) | ||
| if _, err := fmt.Fprintf(os.Stdout, "Template: %s\n", filepath.Join(wd, "config.example.yaml")); err != nil { | ||
| log.Errorf("failed to print config template path: %v", err) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
This block introduces several undefined symbols that will cause compilation errors:
showConfigPathsis used but not declared as a variable or flag.resolveDefaultConfigPathWithCandidatesis called but not defined.printConfigCandidatesis called but not defined.
It appears that the implementation of these helpers and the corresponding flag definition were omitted from this PR.
| govet: | ||
| # Report about shadowed variables | ||
| disable: | ||
| - shadow |
There was a problem hiding this comment.
The shadow check is explicitly disabled within govet. Given that the pull request description mentions fixing variable shadowing issues (e.g., 'claude token path variable shadowing'), it is highly recommended to enable this check to prevent similar regressions in the future.
References
- Standard Go practice encourages using tools like
govetwith theshadowanalyzer enabled to detect accidental variable shadowing. (link) - Shadowing variables can lead to subtle bugs where a local variable unintentionally hides a variable in an outer scope, making the code harder to reason about and maintain.
2 participants
Fix recent compile regressions from generated executor/transpiler edits: mapModelToKiro syntax error recovery, duplicate GitHub Copilot method removal, Codex websocket logger signature fix, and claude token path variable shadowing.