Conversation
- 使用 Justfile 替代 Makefile - 将 Go 代码移动到项目根目录，删除 go/ 子目录 BREAKING CHANGE:废弃 shell 脚本，改用 Go 实现所有功能
- 新增 .goreleaser.yaml、release.yml、compose.goreleaser.yaml 和 .env.goreleaser.example，统一配置 GitHub Release 与 Homebrew Tap 发布 - 将 build-artifact 工作流改为 GoReleaser snapshot 构建，移除手动平台选择参数并补充 fetch-depth: 0 - 删除旧的 prod-release 工作流，justfile 从手写多平台交叉编译改为 release-snapshot、release-dry、release 命令，同时保留本地快速 build - 更新 .gitignore 与 CLAUDE.md，补充新的 CI/CD 流程和发布所需变量说明 原因：统一本地与 CI 的构建发布链路，降低多平台构建脚本维护成本，并提升发布产物、校验与变更日志的一致性。
更新 TODO 文件
- 在 Run 中新增 defer 收尾逻辑，统一执行 Gist 日志上传、最终通知发送与 notifier.Wait - 引入 finalErr 和 stopErrMsg 汇总退出状态，避免早退分支遗漏日志上传或通知不一致 - 调整测试用例，验证服务停止失败时仍会上传一次日志且 success=false，提升失败场景可观测性
|
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:
📝 WalkthroughWalkthroughMajor refactor migrating the project from shell scripts to a Go-based binary (internal/yewresin), introducing GoReleaser-driven release tooling and workflows, adding a VitePress documentation site, removing legacy shell build tooling, and adding example goreleaser env and compose helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Git as GitHub
participant GA as GitHub Actions
participant GR as GoReleaser
participant GH as GitHub Releases
Dev->>Git: push to main or tag v*
Git->>GA: trigger CI / docs / release workflows
GA->>GR: run goreleaser (snapshot or release) with env from `.env.goreleaser` / secrets
GR->>GH: publish artifacts, create release, (optionally) update Homebrew tap
GH-->>Dev: release published / Pages updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 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)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request completes the migration of the YewResin backup tool from a modular Bash script to a Go-based implementation. It includes the addition of GoReleaser for cross-platform builds, a new documentation site using VitePress, and updates to the core backup orchestration logic. My feedback focuses on improving error handling in the Run method by suggesting the use of named return values to ensure consistent notification behavior, enhancing error reporting in notifications, and utilizing errors.Join for better error aggregation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 457b40a4bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/yewresin/orchestrator.go (1)
115-119:⚠️ Potential issue | 🔴 CriticalBug: Lock acquisition failure triggers incorrect "备份成功" notification.
When
acquireLock()fails at line 117, the function returns early. However, the deferred function (lines 90-113) still executes. At this point,finalErrisnilandstopErrMsgis empty, so the switch at line 101-110 falls through to the default case and sends "✅ 备份成功" — even though the backup never actually ran.Similarly, if
DiscoverServices()fails at line 123, the same incorrect notification occurs.🐛 Proposed fix: Set finalErr before early returns
// 1. 获取锁 if err := o.acquireLock(); err != nil { + finalErr = err return err } defer o.releaseLock() // 2. 发现并分类服务 services, err := o.docker.DiscoverServices() if err != nil { - return fmt.Errorf("发现服务失败: %w", err) + finalErr = fmt.Errorf("发现服务失败: %w", err) + return finalErr }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/yewresin/orchestrator.go` around lines 115 - 119, The deferred cleanup/notification uses finalErr and stopErrMsg but early returns from acquireLock() or DiscoverServices() leave finalErr nil, causing a false "✅ 备份成功" notification; update the error paths in the function so that before any early return after calling acquireLock() or DiscoverServices() you assign the encountered error to finalErr (and set stopErrMsg when appropriate) so the deferred function sees the real error state, then return that error; ensure acquireLock()/DiscoverServices() error branches set finalErr (and stopErrMsg if relevant) prior to returning so releaseLock()/defer notification logic behaves correctly.
🧹 Nitpick comments (4)
internal/yewresin/logger_test.go (1)
31-31: 建议断言Write的返回值，避免静默回归。Line 31 和 Line 43 当前忽略了
n, err，建议和前面的写入一样做断言，保证io.Writer合约持续被测试到。♻️ Proposed test hardening
- lc.Write([]byte("world")) + n, err = lc.Write([]byte("world")) + if err != nil || n != 5 { + t.Fatalf("Write: n=%d err=%v", n, err) + } @@ - lc.Write([]byte("multi")) + n, err := lc.Write([]byte("multi")) + if err != nil || n != 5 { + t.Fatalf("Write: n=%d err=%v", n, err) + }Also applies to: 43-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/yewresin/logger_test.go` at line 31, The test currently calls lc.Write([]byte("world")) (and a similar call at the later spot) but ignores the returned n, err; update the test to capture and assert both return values to enforce the io.Writer contract: call lc.Write(...) into variables (n, err), assert err == nil (or expected error) and assert n == len([]byte("world")); do this for the lc.Write call referenced and the second call around the other occurrence so both write results are validated.docs/guide/development.md (1)
11-30: Add language specifier to fenced code block.The project structure code block should have a language specifier for consistency and to satisfy markdown linting rules. Use
textorplaintextfor directory structure blocks.📝 Proposed fix
-``` +```text YewResin/ ├── main.go # 程序入口，CLI 参数解析🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/development.md` around lines 11 - 30, Add a language specifier to the fenced code block that contains the project tree (the block starting with the triple backticks and "YewResin/"); change the opening fence from ``` to ```text (or ```plaintext) so the directory structure is treated as plain text and satisfies markdown linting and consistency requirements.CLAUDE.md (1)
33-55: Add language specifier to fenced code block.Per markdownlint MD040, the directory structure code block should have a language specifier. Use
textfor consistency with other documentation.📝 Proposed fix
-``` +```text YewResin/ ├── main.go # 程序入口，CLI 参数解析🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 33 - 55, Update the fenced directory-structure code block in CLAUDE.md to include a language specifier by changing the opening fence from ``` to ```text so the block becomes a labeled text code block; locate the directory tree block (the YewResin/ listing) and only modify the opening fence to ```text to satisfy markdownlint MD040 while keeping the content unchanged.internal/yewresin/docker_test.go (1)
175-187: Consider adding a compose file to the test directory.This test creates a
DockerManagerwithdryRun: falsebut tests services that aren't running. While the test passes because non-running services are skipped early, if the skip logic changes, the test might fail due to missing compose files inbaseDir.For robustness, consider either:
- Setting
dryRun: truesince the test's intent is to verify skip behavior- Adding a compose file to the temp directory as done in other tests
♻️ Option 1: Use dry-run mode
func TestStopStartNotRunning(t *testing.T) { baseDir := t.TempDir() - dm := NewDockerManager(baseDir, false, time.Second) + dm := NewDockerManager(baseDir, true, time.Second) svc := &Service{Name: "idle", Path: baseDir, Running: false}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/yewresin/docker_test.go` around lines 175 - 187, The test TestStopStartNotRunning constructs a DockerManager with dryRun=false and relies on skipping non-running services; to make the test robust either construct the manager with dryRun=true (e.g., call NewDockerManager(baseDir, true, time.Second)) so no real compose files are required, or create a minimal compose file in baseDir before calling NewDockerManager (so Start/Stop won't fail if skip logic changes); update the test to use one of these approaches and keep references to TestStopStartNotRunning, NewDockerManager/DockerManager, Service, Stop, and Start so the intent remains clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.goreleaser.example:
- Around line 8-10: The dotenv-linter flagged an UnorderedKey for the
environment keys; reorder the three entries so they match the linter's expected
(alphabetical) order by moving HOMEBREW_TAP_NAME and HOMEBREW_TAP_OWNER before
TAP_GITHUB_TOKEN, ensuring the variables TAP_GITHUB_TOKEN, HOMEBREW_TAP_OWNER,
and HOMEBREW_TAP_NAME appear in the correct sorted order (reference the keys
TAP_GITHUB_TOKEN, HOMEBREW_TAP_OWNER, HOMEBREW_TAP_NAME to locate and reorder
them).
In @.github/workflows/docs.yml:
- Around line 26-40: The workflow uses npm commands for the docs job but the
repo uses pnpm (no package-lock.json); update the "Install dependencies" and
"Build with VitePress" steps under working-directory: docs to use pnpm (replace
run: npm ci with a pnpm install using the lockfile, e.g., pnpm install
--frozen-lockfile, and replace run: npm run build with pnpm build or pnpm run
build), and add a setup step to ensure pnpm is available (e.g., uses:
pnpm/action-setup@v2) before the install step so the "Install dependencies" and
"Build with VitePress" steps run with pnpm.
In @.goreleaser.yaml:
- Around line 34-37: The release config currently lists ".env.example" under the
files key in .goreleaser.yaml but that file isn't present in the repo root;
either add a real .env.example file at the repository root with the intended
contents, or remove the ".env.example" entry from the files array in
.goreleaser.yaml (the unique identifier to edit is the files: list containing
LICENSE, README.md, .env.example) so goreleaser no longer fails when packaging
releases.
In `@docs/guide/docs-site.md`:
- Around line 7-24: Update the bare fenced code block that shows the directory
tree (the block starting with ``` and containing the docs/ directory listing) to
specify a language of "text" (replace the opening ``` with ```text) so the
markdown linter rule MD040 is satisfied; leave the block contents unchanged and
ensure the closing triple-backticks remain.
In `@docs/guide/getting-started.md`:
- Line 107: The recovery guide link "恢复指南" currently uses a relative path
"guide/recovery" which resolves incorrectly from this page; update the link
target in docs/guide/getting-started.md for that anchor to an absolute or
correct relative path (e.g., "/guide/recovery" or "./recovery") so the link
resolves properly—locate the markdown link text "恢复指南" and replace the href
"guide/recovery" with the chosen correct path.
In `@docs/guide/gist-logging.md`:
- Around line 35-39: The fenced code block that starts with ``` and contains the
URL "https://gist.github.com/username/abc123def456789" is missing a language tag
which triggers MD040; update that fence to include a language (e.g., add "text"
after the opening ``` so it reads ```text) to explicitly mark the block's
language and resolve the lint warning.
- Around line 55-63: Update the docs to remove the outdated instruction that
`jq` is required and instead state that no extra dependency is needed because
JSON is handled in Go; edit the section around the current `jq` install block
and replace it with a short "依赖检查" paragraph saying the internal implementation
calls the GitHub Gist API directly, referencing the JSON handling implemented in
internal/yewresin/gist.go so readers know the code (internal/yewresin/gist.go)
performs JSON processing without `jq`.
In `@docs/guide/recovery.md`:
- Around line 9-17: Keep the Kopia APT repo URL as shown
(http://packages.kopia.io/apt/) since the GPG-signed key fetched over HTTPS
provides security; do not change that line. Replace the insecure rclone
installation line that pipes a remote script ("curl
https://rclone.org/install.sh | sudo bash") with a safe, explicit procedure:
instruct users to download the precompiled rclone archive (reference the rclone
release tarball), verify the checksum/signature, inspect contents, extract the
rclone binary, and install it with appropriate ownership and permissions (move
binary to /usr/local/bin and chmod +x), and provide the verification and
extraction steps in the doc so users can inspect before execution.
In `@docs/guide/scheduling.md`:
- Around line 5-13: The fenced code block showing the Cron format currently has
no language identifier, triggering markdownlint MD040; update the opening fence
(the triple backticks that start the block containing the Cron diagram and "* *
* * *") to include a language identifier such as "text" or "cron" so the block
becomes ```text (or ```cron) to silence the linter while preserving the diagram
and content.
In `@docs/reference/configuration.md`:
- Line 24: The docs currently state LOG_FILE defaults to "程序同目录下 yewresin.log"
but the actual default in code is empty; update the table entry for `LOG_FILE`
in docs/reference/configuration.md to reflect the real default (empty string) to
match getEnvDefault("LOG_FILE", "") in internal/yewresin/config.go and ensure
the description clarifies that an empty value means no log file is used (or logs
go to stdout).
---
Outside diff comments:
In `@internal/yewresin/orchestrator.go`:
- Around line 115-119: The deferred cleanup/notification uses finalErr and
stopErrMsg but early returns from acquireLock() or DiscoverServices() leave
finalErr nil, causing a false "✅ 备份成功" notification; update the error paths in
the function so that before any early return after calling acquireLock() or
DiscoverServices() you assign the encountered error to finalErr (and set
stopErrMsg when appropriate) so the deferred function sees the real error state,
then return that error; ensure acquireLock()/DiscoverServices() error branches
set finalErr (and stopErrMsg if relevant) prior to returning so
releaseLock()/defer notification logic behaves correctly.
---
Nitpick comments:
In `@CLAUDE.md`:
- Around line 33-55: Update the fenced directory-structure code block in
CLAUDE.md to include a language specifier by changing the opening fence from ```
to ```text so the block becomes a labeled text code block; locate the directory
tree block (the YewResin/ listing) and only modify the opening fence to ```text
to satisfy markdownlint MD040 while keeping the content unchanged.
In `@docs/guide/development.md`:
- Around line 11-30: Add a language specifier to the fenced code block that
contains the project tree (the block starting with the triple backticks and
"YewResin/"); change the opening fence from ``` to ```text (or ```plaintext) so
the directory structure is treated as plain text and satisfies markdown linting
and consistency requirements.
In `@internal/yewresin/docker_test.go`:
- Around line 175-187: The test TestStopStartNotRunning constructs a
DockerManager with dryRun=false and relies on skipping non-running services; to
make the test robust either construct the manager with dryRun=true (e.g., call
NewDockerManager(baseDir, true, time.Second)) so no real compose files are
required, or create a minimal compose file in baseDir before calling
NewDockerManager (so Start/Stop won't fail if skip logic changes); update the
test to use one of these approaches and keep references to
TestStopStartNotRunning, NewDockerManager/DockerManager, Service, Stop, and
Start so the intent remains clear.
In `@internal/yewresin/logger_test.go`:
- Line 31: The test currently calls lc.Write([]byte("world")) (and a similar
call at the later spot) but ignores the returned n, err; update the test to
capture and assert both return values to enforce the io.Writer contract: call
lc.Write(...) into variables (n, err), assert err == nil (or expected error) and
assert n == len([]byte("world")); do this for the lc.Write call referenced and
the second call around the other occurrence so both write results are validated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79542c3b-b7c5-4a6d-8e1e-b7ee92bbf67f
⛔ Files ignored due to path filters (2)
docs/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlgo.sumis excluded by!**/*.sum
📒 Files selected for processing (54)
.env.goreleaser.example.github/workflows/build-artifact.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/go-build-artifact.yml.github/workflows/go-prod-release.yml.github/workflows/prod-release.yml.github/workflows/release.yml.gitignore.goreleaser.yamlCLAUDE.mdMakefileREADME.mdTODO.mdcompose.goreleaser.yamldocs/.vitepress/config.tsdocs/guide/development.mddocs/guide/docs-site.mddocs/guide/getting-started.mddocs/guide/gist-logging.mddocs/guide/how-it-works.mddocs/guide/recovery.mddocs/guide/scheduling.mddocs/index.mddocs/package.jsondocs/pnpm-workspace.yamldocs/reference/configuration.mdgo.modgo/.gitignorego/Makefileinternal/yewresin/backup.gointernal/yewresin/config.gointernal/yewresin/config_test.gointernal/yewresin/docker.gointernal/yewresin/docker_test.gointernal/yewresin/gist.gointernal/yewresin/logger.gointernal/yewresin/logger_test.gointernal/yewresin/notify.gointernal/yewresin/orchestrator.gointernal/yewresin/orchestrator_test.gojustfilemain.gomain_test.gosrc/00-header.shsrc/01-logging.shsrc/02-args.shsrc/03-config.shsrc/04-utils.shsrc/05-notification.shsrc/06-gist.shsrc/07-dependencies.shsrc/08-services.shsrc/09-main.sh
💤 Files with no reviewable changes (16)
- src/04-utils.sh
- src/01-logging.sh
- src/00-header.sh
- .github/workflows/prod-release.yml
- .github/workflows/go-prod-release.yml
- Makefile
- go/.gitignore
- src/02-args.sh
- .github/workflows/go-build-artifact.yml
- src/05-notification.sh
- src/09-main.sh
- src/03-config.sh
- src/06-gist.sh
- go/Makefile
- src/07-dependencies.sh
- src/08-services.sh
修正环境变量示例按照字母顺序排列
- 补充配置加载顺序、必填项校验和 --config 默认行为，更新 LOG_FILE 默认值及日志持久化说明 - 在 README 增加临时环境变量运行示例，帮助快速启动 - 更新 Gist 日志指南，移除 jq 安装步骤并说明由程序内置实现直接调用 Gist API - 修正恢复指南与入门文档中的链接、安装说明和 text 代码块标注，减少渲染与使用歧义 使文档与当前程序行为保持一致，降低配置错误和上手成本。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/guide/getting-started.md (2)
13-13: Consider a safer installation method.Piping
curloutput directly tosudo bashcan be risky as it executes code without inspection. Consider suggesting users download the script first and inspect it, or prefer official package manager installations where available.🛡️ Safer alternative approach
-curl https://rclone.org/install.sh | sudo bash +# Download and inspect before running +curl -O https://rclone.org/install.sh +# Review the script, then run: +sudo bash install.shOr mention package manager installation if available for the target OS.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/getting-started.md` at line 13, Replace the unsafe direct-pipe command "curl https://rclone.org/install.sh | sudo bash" with guidance to download the installer first and inspect it (e.g., "curl -O" or "wget" to fetch the script, verify checksum/signature, review contents) before running with sudo, and also mention installing via the system package manager or official distribution packages for the user's OS as the preferred safer alternative; update the single-line example and surrounding text to recommend these safer workflows.
34-44: Add checksum verification step.Downloading binaries without verifying checksums leaves users vulnerable to corrupted or tampered files. GoReleaser typically generates checksums for releases. Consider adding a verification step.
🔒 Suggested addition for checksum verification
Add after the download commands:
# 验证下载文件（可选但推荐） wget https://github.com/YewFence/YewResin/releases/latest/download/checksums.txt sha256sum -c checksums.txt --ignore-missing🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/getting-started.md` around lines 34 - 44, Add a checksum verification step to the download instructions in getting-started.md: after the wget lines, instruct users to download the generated checksums file (checksums.txt) from the release and verify the downloaded binary(s) with a checksum verifier (e.g., sha256sum -c checksums.txt --ignore-missing or the platform-appropriate shasum/PowerShell command), and include a brief note for Windows users to verify the .exe similarly.
🤖 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/build-artifact.yml:
- Around line 38-43: The GoReleaser action in the build-artifact workflow uses
version: "~> v2" which mismatches the release workflow's v3; update the
goreleaser/goreleaser-action configuration in this file by changing the version
field from "~> v2" to the same v3 spec used in .github/workflows/release.yml so
both workflows use the same GoReleaser major version (reference: the
goreleaser/goreleaser-action@v6 step and the version: "~> v2" entry).
---
Nitpick comments:
In `@docs/guide/getting-started.md`:
- Line 13: Replace the unsafe direct-pipe command "curl
https://rclone.org/install.sh | sudo bash" with guidance to download the
installer first and inspect it (e.g., "curl -O" or "wget" to fetch the script,
verify checksum/signature, review contents) before running with sudo, and also
mention installing via the system package manager or official distribution
packages for the user's OS as the preferred safer alternative; update the
single-line example and surrounding text to recommend these safer workflows.
- Around line 34-44: Add a checksum verification step to the download
instructions in getting-started.md: after the wget lines, instruct users to
download the generated checksums file (checksums.txt) from the release and
verify the downloaded binary(s) with a checksum verifier (e.g., sha256sum -c
checksums.txt --ignore-missing or the platform-appropriate shasum/PowerShell
command), and include a brief note for Windows users to verify the .exe
similarly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d59376e-d681-4f94-a7f7-0e6d4da865bf
📒 Files selected for processing (12)
.env.goreleaser.example.github/workflows/build-artifact.yml.github/workflows/ci.yml.github/workflows/docs.ymlREADME.mddocs/guide/docs-site.mddocs/guide/getting-started.mddocs/guide/gist-logging.mddocs/guide/recovery.mddocs/guide/scheduling.mddocs/reference/configuration.mdinternal/yewresin/orchestrator.go
✅ Files skipped from review due to trivial changes (9)
- .env.goreleaser.example
- .github/workflows/ci.yml
- docs/reference/configuration.md
- docs/guide/scheduling.md
- .github/workflows/docs.yml
- docs/guide/docs-site.md
- README.md
- docs/guide/recovery.md
- docs/guide/gist-logging.md
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/yewresin/orchestrator.go
- 在 CI 中新增 go mod tidy 检查并校验 go.mod/go.sum 无差异，防止依赖漂移导致构建不一致 - 升级 pnpm 到 v10；docs 工作流补充 pnpm 缓存与锁文件路径，提升构建稳定性与效率 - 将 release 与 compose 的 GoReleaser 统一为 v2，并同步 .goreleaser.yaml 为 v2 配置（go mod download、archives.formats、homebrew_casks），确保发布链路兼容可用
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.lefthook/pre-commit/pinact-check.sh (1)
3-3: Token setup failure is silently ignored.If
ghtkn getfails (tool not installed, auth expired, network issue), the pipeline continues silently andpinact run --checkmay fail cryptically due to a missing/invalid token. Consider checking the exit status:🛡️ Proposed fix to handle token setup failure
-ghtkn get | pinact token set -stdin +if ! ghtkn get | pinact token set -stdin; then + echo "⚠️ Warning: Failed to set GitHub token. pinact may fail or hit rate limits." +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.lefthook/pre-commit/pinact-check.sh at line 3, The current pipeline line "ghtkn get | pinact token set -stdin" ignores failures from ghtkn or pinact; update the pre-commit script to capture and check the exit status of both commands (the ghtkn token retrieval and the pinact token set) and abort with a clear error log if either fails: run ghtkn get into a variable or temp file, check its exit code and non-empty output before piping to pinact token set -stdin, then check pinact's exit code and exit non-zero with an explanatory message if it fails (referencing the ghtkn get and pinact token set steps in the script).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.lefthook/pre-commit/pinact-check.sh:
- Line 3: The current pipeline line "ghtkn get | pinact token set -stdin"
ignores failures from ghtkn or pinact; update the pre-commit script to capture
and check the exit status of both commands (the ghtkn token retrieval and the
pinact token set) and abort with a clear error log if either fails: run ghtkn
get into a variable or temp file, check its exit code and non-empty output
before piping to pinact token set -stdin, then check pinact's exit code and exit
non-zero with an explanatory message if it fails (referencing the ghtkn get and
pinact token set steps in the script).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17390485-34fe-47fa-b787-6df5f2fda724
📒 Files selected for processing (7)
.github/workflows/build-artifact.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/release.yml.lefthook/pre-commit/pinact-check.shTODO.mdlefthook.yml
✅ Files skipped from review due to trivial changes (5)
- TODO.md
- lefthook.yml
- .github/workflows/release.yml
- .github/workflows/ci.yml
- .github/workflows/docs.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/guide/development.md (1)
83-88: Use neutral placeholders for tokens in docs examples.Using
ghp_...-style examples is easy to misread as real-token format and can trip secret scanners in copied snippets.🔐 Suggested patch
-GITHUB_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxx -TAP_GITHUB_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxx # Homebrew Tap 仓库的 PAT +GITHUB_TOKEN=<your_github_token> +TAP_GITHUB_TOKEN=<your_homebrew_tap_token> # Homebrew Tap 仓库的 PAT HOMEBREW_TAP_OWNER=YewFence HOMEBREW_TAP_NAME=homebrew-tap🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/development.md` around lines 83 - 88, Replace the real-looking ghp_* placeholders in the example environment block with neutral, non-GitHub-specific placeholders for the variables GITHUB_TOKEN and TAP_GITHUB_TOKEN (and keep HOMEBREW_TAP_OWNER / HOMEBREW_TAP_NAME as-is); update the docs so the tokens read as generic placeholders like TOKEN_PLACEHOLDER or YOUR_TOKEN to avoid triggering secret scanners and reduce confusion when users copy the snippet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guide/development.md`:
- Around line 14-33: The Markdown code fence for the project tree (the block
starting with "YewResin/") lacks a language tag and triggers MD040; update the
opening triple-backtick to include a language identifier (e.g., ```text) so the
fence becomes ```text and leave the rest of the block unchanged to silence the
MD040 lint warning.
---
Nitpick comments:
In `@docs/guide/development.md`:
- Around line 83-88: Replace the real-looking ghp_* placeholders in the example
environment block with neutral, non-GitHub-specific placeholders for the
variables GITHUB_TOKEN and TAP_GITHUB_TOKEN (and keep HOMEBREW_TAP_OWNER /
HOMEBREW_TAP_NAME as-is); update the docs so the tokens read as generic
placeholders like TOKEN_PLACEHOLDER or YOUR_TOKEN to avoid triggering secret
scanners and reduce confusion when users copy the snippet.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c8ef03f-a29e-4317-8ff4-ee47a9bf080d
📒 Files selected for processing (2)
.lefthook/pre-commit/pinact-check.shdocs/guide/development.md
✅ Files skipped from review due to trivial changes (1)
- .lefthook/pre-commit/pinact-check.sh
进行 go mod verify / govulncheck 以进行安全审计
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
CI/CD & Distribution
Documentation