Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAdds a CI step that publishes and removes a layer, bumps Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/index.ts (1)
331-344:⚠️ Potential issue | 🔴 CriticalReturned path may not exist after forced
.zipsuffix.On Line [344], the function mutates the returned path string but does not ensure the downloaded file is written with that exact name. This can return a non-existent file path.
Proposed fix
- const parsedPathName = path.parse(urlPath).name; - const filename = path.basename(urlPath) || `downloaded_code_${Date.now()}`; + const parsed = path.parse(urlPath); + const baseName = parsed.name || `downloaded_code_${Date.now()}`; + const filename = parsed.ext ? parsed.base : `${baseName}.zip`; downloadPath = path.join(tempDir, filename); await downloads(url, { dest: tempDir, - filename: parsedPathName, + filename, extract: false, }); logger.debug(`Downloaded file to: ${downloadPath}`); - // 返回下载文件路径，由主流程决定是否需要压缩 - return downloadPath.endsWith('.zip') ? downloadPath : `${downloadPath}.zip`; + // 返回真实下载文件路径 + return downloadPath;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/index.ts` around lines 331 - 344, The code returns downloadPath with a forced .zip suffix which may not match the actual downloaded file name; update the download flow so the saved file and returned path always match: compute the desired saved filename (use parsedPathName or filename plus desired extension), pass that exact filename/path into downloads (or save into downloadPath directly) and, if needed, after downloads finishes rename/move the saved file to downloadPath (or create a zip at downloadPath) before returning; adjust references to parsedPathName, filename, downloadPath and the downloads call so the returned path points to an existing file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/e2e/ci-mac-linux.sh`:
- Around line 160-162: Replace the hardcoded layer name and add non-interactive
removal: generate a unique layer name (e.g., using timestamp or CI JOB_ID)
instead of the literal `--layer-name test` when calling `s layer publish` and
use that same generated name for `s layer remove`; also add the auto-confirm
flag `-y` to the `s layer remove` invocation so the CI won't wait for
interactive confirmation. Ensure the generated name is stored in a variable and
referenced in both `s layer publish` and `s layer remove -y`.
---
Outside diff comments:
In `@src/utils/index.ts`:
- Around line 331-344: The code returns downloadPath with a forced .zip suffix
which may not match the actual downloaded file name; update the download flow so
the saved file and returned path always match: compute the desired saved
filename (use parsedPathName or filename plus desired extension), pass that
exact filename/path into downloads (or save into downloadPath directly) and, if
needed, after downloads finishes rename/move the saved file to downloadPath (or
create a zip at downloadPath) before returning; adjust references to
parsedPathName, filename, downloadPath and the downloads call so the returned
path points to an existing file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3c496a4-78b5-412e-a6d0-9ab97a7cf257
📒 Files selected for processing (3)
__tests__/e2e/ci-mac-linux.shpublish.yamlsrc/utils/index.ts
__tests__/e2e/ci-mac-linux.sh
Outdated
Comment on lines
+160
to
+162
| echo "test layer code support http" | ||
| s layer publish --layer-name test --code https://images.devsapp.cn/bin/supergateway --compatible-runtime custom.debian12 | ||
| s layer remove --layer-name test |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify remove commands that may be interactive
rg -nP --type=sh '\bs\s+layer\s+remove\b' __tests__/e2e/ci-mac-linux.sh
# Verify hard-coded layer names in CI
rg -nP --type=sh '--layer-name\s+test\b' __tests__/e2e/ci-mac-linux.shRepository: devsapp/fc3
Length of output: 532
🏁 Script executed:
# Get exact content at lines 160-162
sed -n '160,162p' __tests__/e2e/ci-mac-linux.sh
# Check if RANDSTR is defined in the script
grep -n "RANDSTR" __tests__/e2e/ci-mac-linux.sh
# Look for any layer remove commands with flags for context
grep -n "layer remove" __tests__/e2e/ci-mac-linux.shRepository: devsapp/fc3
Length of output: 1249
🏁 Script executed:
# Search for all layer remove commands across the codebase
rg "s layer remove" -A 1 -B 1
# Check if -y flag is used with layer remove elsewhere
rg "layer remove.*-y" --type=shRepository: devsapp/fc3
Length of output: 875
CI layer commands use hardcoded name and lack auto-confirmation flag.
Line 161 uses static --layer-name test, risking collisions with concurrent runs or leftovers. Line 162 omits -y, which can cause CI to hang waiting for interactive confirmation. Other test suites (command-api/run, run-windows.ps1) use dynamic layer names and include -y with layer remove.
Proposed fix
echo "test layer code support http"
-s layer publish --layer-name test --code https://images.devsapp.cn/bin/supergateway --compatible-runtime custom.debian12
-s layer remove --layer-name test
+layer_name="test-${RANDSTR}-${RANDOM}"
+s layer publish --layer-name "${layer_name}" --code https://images.devsapp.cn/bin/supergateway --compatible-runtime custom.debian12
+s layer remove --layer-name "${layer_name}" -y🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@__tests__/e2e/ci-mac-linux.sh` around lines 160 - 162, Replace the hardcoded
layer name and add non-interactive removal: generate a unique layer name (e.g.,
using timestamp or CI JOB_ID) instead of the literal `--layer-name test` when
calling `s layer publish` and use that same generated name for `s layer remove`;
also add the auto-confirm flag `-y` to the `s layer remove` invocation so the CI
won't wait for interactive confirmation. Ensure the generated name is stored in
a variable and referenced in both `s layer publish` and `s layer remove -y`.
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
Bug Fixes
Chores
Tests