Conversation
Summary of ChangesHello @sususu98, 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! 此拉取请求引入了一个新的命令行工具,旨在高效管理和清理系统中与特定供应商相关的会话绑定。该工具提供了多种筛选机制和交互式操作选项,并支持干运行模式以确保操作安全。此外,还包含了相应的Docker Compose配置,以便于在容器化环境中部署和执行此维护脚本。 Highlights
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
|
| env_file: | ||
| - .env | ||
| environment: | ||
| DSN: postgresql://${DB_USER:-postgres}:${DB_PASSWORD:-postgres}@postgres:5432/${DB_NAME:-claude_code_hub} |
| }, | ||
| lazyConnect: true, | ||
| }; | ||
|
|
||
| if (redisUrl.startsWith("rediss://")) { | ||
| try { |
There was a problem hiding this comment.
在 createRedisClient 函数中,当处理 rediss:// URL 时,如果 new URL(redisUrl) 解析失败,catch 块会将 options.tls 设置为空对象 {}。这可能会导致需要 SNI (Server Name Indication) 的 Redis 服务(如 Upstash)连接失败,因为 hostname 没有被显式设置。建议在 catch 块中记录一个明确的警告,而不是静默地回退,以帮助排查潜在的连接问题。
try {
const url = new URL(redisUrl);
options.tls = { host: url.hostname };
} catch (e) {
console.warn(`[Redis] 无法从 REDIS_URL 解析主机名,TLS 连接可能因缺少 SNI 而失败: ${e instanceof Error ? e.message : String(e)}`);
options.tls = {};
}| console.log("\n已退出。"); | ||
| return; | ||
| } | ||
| // 简单处理:重新开始 | ||
| console.log("\n请重新运行脚本。"); | ||
| return; | ||
| } | ||
| const filteredProviders = | ||
| selectedType === "all" | ||
| ? allProviders | ||
| : allProviders.filter((p) => p.providerType === selectedType); |
|
会话绑定清理工具使用说明 功能概述 scripts/clear-session-bindings.ts 是一个用于清除指定供应商会话绑定的 CLI 工具。当你需要将某些供应商下线或重置其会话状态时,可以使用此工具批量清理 Redis 中的会话数据。 运行方式 方式一:本地运行(需要 Bun 环境) bun run scripts/clear-session-bindings.ts [选项] 方式二:Docker 容器运行(推荐生产环境) docker compose -f docker-compose.yaml -f deploy/docker-compose.tools.yaml 使用模式
不带参数运行,进入交互式菜单: bun run scripts/clear-session-bindings.ts 交互式菜单提供以下选项:
按优先级筛选 清除优先级小于 10 的所有供应商的会话绑定bun run scripts/clear-session-bindings.ts --priority 10 仅清除 Claude 类型、优先级小于 5 的供应商bun run scripts/clear-session-bindings.ts --priority 5 --type claude 指定供应商 ID 清除 ID 为 1, 2, 3 的供应商bun run scripts/clear-session-bindings.ts --id 1,2,3 按名称模糊匹配 清除名称包含 "cubence" 的供应商bun run scripts/clear-session-bindings.ts --name "cubence" 命令行参数
使用示例 预览模式:查看将清理哪些内容(不实际删除)bun run scripts/clear-session-bindings.ts --priority 10 --dry-run 静默模式:跳过确认直接执行bun run scripts/clear-session-bindings.ts --id 1,2,3 --yes Docker 环境中执行docker compose -f docker-compose.yaml -f deploy/docker-compose.tools.yaml 注意事项
|
|
|
||
| if (arg === "--priority" || arg === "-p") { | ||
| const nextValue = args[++i]; | ||
| if (!nextValue) throw new Error("--priority 需要一个数值参数"); |
There was a problem hiding this comment.
MEDIUM Array bounds violation in argument parsing
Why this is a problem: When parsing command-line arguments, the code uses args[++i] which increments i before accessing the array. If a flag like --priority or --type is the last argument without a value, the code will read past the end of the array (args[args.length] returns undefined). While the subsequent validation catches this, the pre-increment pattern is unsafe and could mask the actual issue location in error messages.
Suggested fix:
if (arg === "--priority" || arg === "-p") {
i++; // Increment after the check
const nextValue = args[i];
if (!nextValue || i >= args.length) throw new Error("--priority 需要一个数值参数");
priorityValue = Number.parseInt(nextValue, 10);
if (Number.isNaN(priorityValue)) throw new Error("--priority 必须是整数");
}
// Apply the same pattern to --id, --name, and --type| if (Number.isNaN(priorityValue)) throw new Error("--priority 必须是整数"); | ||
| } else if (arg.startsWith("--priority=")) { | ||
| priorityValue = Number.parseInt(arg.split("=")[1], 10); | ||
| if (Number.isNaN(priorityValue)) throw new Error("--priority 必须是整数"); |
There was a problem hiding this comment.
MEDIUM Missing validation for split("=") operations
Why this is a problem: When parsing arguments like --priority= or --name=, the code uses .split("=")[1] without verifying that the split result contains at least 2 elements. If a user provides a malformed argument (e.g., --priority= with no value after the equals sign, or --name==value with double equals), the code will either get an empty string or unexpected behavior, leading to confusing error messages or crashes.
Suggested fix:
} else if (arg.startsWith("--priority=")) {
const parts = arg.split("=");
if (parts.length < 2 || !parts[1]) {
throw new Error("--priority 需要一个数值参数");
}
priorityValue = Number.parseInt(parts[1], 10);
if (Number.isNaN(priorityValue)) throw new Error("--priority 必须是整数");
}
// Apply the same pattern to --id=, --name=, and --type=
ding113
left a comment
There was a problem hiding this comment.
Code Review Summary
This PR adds a session binding cleanup CLI tool and Docker Compose tools configuration. The main TypeScript script (965 lines) is well-structured with proper error handling and resource cleanup. The JSON file changes appear to be a formatting/sync update from LiteLLM. Two medium-severity issues were identified in the argument parsing logic that could lead to confusing error behavior with malformed inputs.
PR Size: XL
- Lines changed: 54,338 (26,619 additions + 27,719 deletions)
- Files changed: 4
Split Recommendation: The majority of changes (~53k lines) are in litellm-prices.json which is an auto-generated data file. Consider:
- Separating the JSON data sync into its own PR
- Keeping the feature implementation (script + docker config + version bump) in a focused PR
This would make reviews more manageable and allow the feature PR to be reviewed/merged independently of the data sync.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Code Quality | 0 | 0 | 2 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Dependencies | 0 | 0 | 0 | 0 |
| Documentation | 0 | 0 | 0 | 0 |
Priority Actions
- Fix argument parsing array bounds (Medium): The
args[++i]pattern inparseCliArgs()could access beyond array bounds. Change to increment after bounds check. - Validate split("=") results (Medium): Arguments like
--priority=,--name=don't validate that split returns a non-empty value after the equals sign.
Review Coverage
- Code quality and correctness
- Security (OWASP Top 10) - Clean
- PR size assessment - XL (recommend splitting)
- Dependency changes - No package.json changes
- Documentation changes - No docs changes
Automated review by Claude AI
…indings.ts The `providerType` parameter was typed as `string` but the Drizzle ORM `eq()` function expects the specific provider type union. Added a type alias and cast to fix the type error. Fixes CI failure in PR #268. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
似乎修改了一些无关文件,到时我单独 pick 一下,晚些合并 |
新增供应商会话绑定清理 CLI 工具: - `scripts/clear-session-bindings.ts`: 交互式或命令行清理工具 - 支持按优先级、供应商 ID、名称模式或类型筛选 - 支持 dry-run 模式预览操作 - 使用 Redis pipeline 批量处理优化性能 - `deploy/docker-compose.tools.yaml`: 用于在容器化环境中运行维护脚本 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add ProviderType type definition with validation - Reorder imports alphabetically - Apply code formatting improvements
Summary
scripts/clear-session-bindings.ts会话绑定清理 CLI 工具deploy/docker-compose.tools.yaml用于在 Docker 环境中运行维护脚本Test plan
--priority参数筛选--id参数指定供应商--name参数模糊匹配--dry-run预览模式🤖 Generated with Claude Code