Refactor/file Add build versioning, environment configuration, and file logging#3
Refactor/file Add build versioning, environment configuration, and file logging#3Embers-of-the-Fire merged 18 commits intomainfrom
Conversation
The root .gitignore was empty. Add ignore rules for common editor swap/backup files (vim, Sublime), IDE directories (.idea, .vscode), and OS-generated artifacts (.DS_Store, Thumbs.db). Per-package ignores (target/, /runtime, etc.) are already handled by their respective .gitignore files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add description, license, repository URL, and publish = false to clarify the package identity and prevent accidental crate publishes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a [profile.release] section with LTO, strip, codegen-units = 1, and opt-level = 3 to produce smaller, faster release binaries. These settings are standard for deployment-oriented Rust services. Add [lints.clippy] section enabling pedantic lints with targeted allows for module_name_repetitions and must_use_candidate, which are overly noisy for this web-service codebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The runtime init method only created the `robots` table, but schema.sql also defines `network_info`. This caused write_network_info / get_network_info queries to fail if the table did not already exist (e.g. fresh deployments without the CI schema bootstrapping step). Mirror the full schema.sql definition so both tables are guaranteed to exist after Database::init(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the confusing double-error pattern (fmt.Errorf assigned to a shadowed err, then nil-checked — which is always non-nil) with a direct fmt.Fprintf to stderr. The error message now goes to stderr instead of stdout, which is the correct stream for diagnostic output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The service required LOG_DIR and created the directory, but only logged to stdout console — the env var was effectively unused. Add a rolling-file appender that writes JSON-encoded log entries to LOG_DIR/service.log, matching the bot's file-logging behaviour. Logs rotate when the file exceeds 10 MB, keeping up to 5 gzipped archives. The default log level is also tightened from Debug to Info for production readiness; debug output remains reachable by lowering the filter at runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The server previously hardcoded "0.0.0.0:3000" as the listen address and "http://localhost:3000/api" as the Swagger server URL. Introduce a BIND_ADDR environment variable (defaulting to 0.0.0.0:3000) that controls both the TCP listener and the Swagger UI server URL. This allows deploying on non-standard ports or binding to a specific interface without code changes. Also adds a startup log line so operators can confirm which address the server is listening on. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a package-level Version variable (default "dev") that can be set at compile time via -ldflags="-X main.Version=...". Also add a --version CLI flag that prints the version and exits, giving operators a simple way to verify which build is deployed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CI builds now embed "ci-<sha>" as the version, and release builds embed the git tag (e.g. "v1.2.3"). This pairs with the new --version flag added to the bot, letting operators verify which exact build is deployed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert the single-arch Go build step into a matrix strategy that produces both linux/amd64 and linux/arm64 binaries. This supports deploying the bot on ARM-based SBCs (common in robotics setups). Add a SHA256SUMS.txt checksums file to the release artifacts so downstream consumers can verify download integrity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add inline comments explaining each configuration field, set default paths to runtime/logs and runtime/storage to match the .gitignore /runtime pattern, and add a copy-to-config.yaml instruction at the top of the file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Automated formatting fixes from cargo fmt: collapsed single-item argument lists and shortened import groupings to satisfy the project max_width = 80 constraint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
Walkthrough引入构建时版本注入与多架构发布构建;扩展 .gitignore;Bot 增加版本标志与配置路径更新;Service 增加可配置绑定地址、文件滚动日志、network_info 表及迁移逻辑;若干日志/格式与内部返回类型调整。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
兔子的诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/bot/config.example.yaml (1)
6-7: 建议在目录项注释中标明“必填,不能为空”。结合当前加载逻辑,
dir为空会在启动时触发目录创建错误。建议在 Line 6-7 和 Line 10-11 的注释里明确“必填且不能为空”,让问题前置到配置阶段。Also applies to: 10-11
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/config.example.yaml` around lines 6 - 7, 在配置示例中针对键名 "dir" 的注释未标明这是必填项且不能为空;请在所有出现 "dir" 的注释(示例中两处描述“Directory for rotating log files (created automatically).”的位置)直接追加“必填,不能为空”的说明,以便使用者在配置阶段注意并避免启动时因空路径触发目录创建错误;同时确保注释文本清晰表述“必填且不能为空(required, non-empty)”以与现有加载逻辑对齐。
🤖 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/release.yml:
- Around line 84-86: The "Generate SHA256 checksums" step currently runs
"sha256sum *" which fails if artifacts/ contains subdirectories; change the step
so it only enumerates regular files (not directories), sorts them
deterministically, and then computes SHA256 checksums into SHA256SUMS.txt;
update the run command in that step to use a file-only finder + deterministic
sort piped into sha256sum (so SHA256SUMS.txt contains checksums for files only
and in a stable order).
In `@packages/service/.env.example`:
- Line 4: 将 .env.example 中的 BIND_ADDR 键移动到 DATABASE_URL 之前以消除 dotenv-linter 的
UnorderedKey 警告:在文件中找到 BIND_ADDR 和 DATABASE_URL,交换它们的位置(确保 BIND_ADDR 出现在
DATABASE_URL 之上),保存并提交更改以消除 CI/本地 lint 噪音。
In `@packages/service/src/database.rs`:
- Around line 29-39: The network_info table's robot_uuid is only a primary key
and needs a FOREIGN KEY to robots(uuid) to prevent orphan rows and support
cascade deletes: update the CREATE TABLE in the network_info creation (the SQL
executed via sqlx::query! in database.rs) to include "FOREIGN KEY(robot_uuid)
REFERENCES robots(uuid) ON DELETE CASCADE". Also ensure SQLite foreign keys are
enabled when opening the DB by updating the connection initialization (the
new(...) constructor that builds self.connection) to use
sqlx::sqlite::SqliteConnectOptions and set .pragma("foreign_keys","ON") (then
connect_with those options to create the SqlitePool). Finally keep using
.execute(&self.connection).await? for the create statement so the constraint is
applied at startup.
In `@packages/service/src/main.rs`:
- Around line 37-43: The OpenAPI server URL is being built from the listen
address (bind_addr) and passed into OpenApiService::server(), which exposes an
unreachable host; change the code that constructs server_url and the call to
OpenApiService::new(...).server(...) to use the relative path "/api" instead of
format!("http://{bind_addr}/api"). Update the variable or inline value used for
api_service so OpenApiService::server("/api") is used (keep bind_addr only for
the actual listener, not for the OpenAPI spec).
---
Nitpick comments:
In `@packages/bot/config.example.yaml`:
- Around line 6-7: 在配置示例中针对键名 "dir" 的注释未标明这是必填项且不能为空;请在所有出现 "dir"
的注释(示例中两处描述“Directory for rotating log files (created
automatically).”的位置)直接追加“必填,不能为空”的说明,以便使用者在配置阶段注意并避免启动时因空路径触发目录创建错误;同时确保注释文本清晰表述“必填且不能为空(required,
non-empty)”以与现有加载逻辑对齐。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c60f5114-dffe-45b0-a8b8-738df121c4a7
📒 Files selected for processing (11)
.github/workflows/ci.yml.github/workflows/release.yml.gitignorepackages/bot/config.example.yamlpackages/bot/main.gopackages/service/.env.examplepackages/service/Cargo.tomlpackages/service/src/constant/env.rspackages/service/src/database.rspackages/service/src/logger.rspackages/service/src/main.rs
Replace the release checksum generation step with a file-only pipeline so merged artifact downloads do not fail when actions/download-artifact creates subdirectories. The new implementation walks the artifact tree, excludes the checksum file itself, and sorts the file list with LC_ALL=C before hashing to keep SHA256SUMS.txt deterministic across runs. Update the GitHub release asset upload step to use the same file-only approach. This prevents directories from being passed to gh release create and keeps the published asset order stable when the artifact layout changes. Co-authored-by: Codex <codex@openai.com>
Reorder the service example environment file to follow the key ordering expected by dotenv-linter. This moves the bind address ahead of the database URL and keeps the remaining entries in a stable alphabetical sequence. Keeping the example file ordered removes recurring UnorderedKey noise from CI and local lint runs, so future configuration changes do not mask real dotenv issues behind avoidable ordering warnings. Co-authored-by: Codex <codex@openai.com>
Enable SQLite foreign key enforcement at connection creation time by switching the pool setup to SqliteConnectOptions::foreign_keys(true). This ensures every pooled connection enforces the relationship between network_info.robot_uuid and robots.uuid instead of relying on a one-off PRAGMA call that would only affect a single connection. Add the missing foreign key definition to both the runtime schema bootstrap and schema.sql so new databases and sqlx validation databases share the same table shape. The relationship now cascades deletes from robots to network_info, preventing orphaned network rows from surviving robot removal. Introduce a lightweight init-time migration for existing deployments whose network_info table predates the foreign key. When the constraint is missing, the service renames the legacy table, recreates it with the new constraint, copies existing rows back under foreign key enforcement, and then drops the legacy copy after a successful transaction. Co-authored-by: Codex <codex@openai.com>
Stop deriving the OpenAPI server URL from the bind address and publish the API base as the relative path /api instead. This keeps the generated spec usable when the service listens on 0.0.0.0 locally and when it is deployed behind a reverse proxy or TLS terminator. Using a relative server entry lets Swagger UI and generated clients resolve the host and scheme from the actual request instead of hard-coding an internal listener address. That avoids exposing unreachable URLs such as http://0.0.0.0:3000/api in the published specification. Co-authored-by: Codex <codex@openai.com>
1895056 to
ee09feb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/service/src/service/connection.rs (1)
105-105: 建议为Unknown分支增加最小告警日志。显式匹配很好,但完全静默会增加排障成本;建议至少记录一次
warn(不打印 payload 细节,避免潜在敏感信息泄露)。可选修改示例
- MessagePayload::Unknown(_) => {} + MessagePayload::Unknown(_) => { + log::warn!("Ignoring unknown payload for session_id: {session_id}"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/service/src/service/connection.rs` at line 105, 在对 MessagePayload 进行匹配的分支里,针对 MessagePayload::Unknown(_) 分支增加一次最小级别的告警日志(例如使用 warn 级别),不要打印或格式化 payload 内容以避免泄漏敏感信息;定位到在 connection.rs 中处理消息的匹配(即匹配到 MessagePayload::Unknown(_) 的分支),调用现有的日志记录器(如 connection 的 logger 或模块级 logger)记录一条简短描述性信息(例如 "Received unknown message payload"),然后保持原有的空实现行为。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/service/src/api.rs`:
- Around line 25-27: The bad_request function currently maps client errors to
GenericResponse::InternalError (500); update the bad_request(Error) ->
GenericResponse implementation to return GenericResponse::BadRequest instead and
keep the same PlainText format with the error message so client errors produce
an HTTP 400; locate the change in the bad_request function and replace
GenericResponse::InternalError with GenericResponse::BadRequest.
---
Nitpick comments:
In `@packages/service/src/service/connection.rs`:
- Line 105: 在对 MessagePayload 进行匹配的分支里,针对 MessagePayload::Unknown(_)
分支增加一次最小级别的告警日志(例如使用 warn 级别),不要打印或格式化 payload 内容以避免泄漏敏感信息;定位到在 connection.rs
中处理消息的匹配(即匹配到 MessagePayload::Unknown(_) 的分支),调用现有的日志记录器(如 connection 的 logger
或模块级 logger)记录一条简短描述性信息(例如 "Received unknown message payload"),然后保持原有的空实现行为。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abbf9cbf-a44a-464b-b302-4747dbcaf222
📒 Files selected for processing (18)
packages/service/src/api.rspackages/service/src/api/action.rspackages/service/src/api/action/update_binary.rspackages/service/src/api/ident.rspackages/service/src/api/stats.rspackages/service/src/database/robot.rspackages/service/src/env.rspackages/service/src/main.rspackages/service/src/service.rspackages/service/src/service/action.rspackages/service/src/service/connection.rspackages/service/src/service/events.rspackages/service/src/service/instructions.rspackages/service/src/service/instructions/fetch_network.rspackages/service/src/service/instructions/sync_robot_name.rspackages/service/src/service/instructions/update_binary.rspackages/service/src/service/instructions/update_metadata.rspackages/service/src/service/message.rs
✅ Files skipped from review due to trivial changes (9)
- packages/service/src/api/action/update_binary.rs
- packages/service/src/service/instructions.rs
- packages/service/src/service.rs
- packages/service/src/env.rs
- packages/service/src/main.rs
- packages/service/src/database/robot.rs
- packages/service/src/service/action.rs
- packages/service/src/api/stats.rs
- packages/service/src/service/events.rs
Co-authored-by: Codex <codex@openai.com>
本次 PR 在构建/发布、运行时配置、日志记录与数据库初始化等方面进行了重构与增强,提升可部署性、可观测性与多架构发布能力。主要变更如下:
构建与发布
Bot(packages/bot)
Service(packages/service)
包与构建配置
其他维护性改动
总体而言,本次提交围绕版本注入与多架构发布、运行时环境可配置性、结构化与文件化日志、以及数据库外键安全迁移进行了系统性改进,同时对若干内部实现与格式化进行了统一与修整,目标是提升可部署性、可维护性与运行时可观测性。