Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMake Licence fields optional and reshape RateLimit into per-category objects; update CLI formatting and tests accordingly. Add a read-only smoke-test script, extend spell-check dictionary, and ignore its result file; update docs (EN/JA) for the changed outputs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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.
Pull request overview
Fixes deserialization failures for bl space licence and bl rate-limit by aligning Rust structs with the actual Backlog API response shapes, plus adds a new smoke-test script to validate read-only commands against a real space.
Changes:
- Make
Licencefields optional where the API may omit them, and update CLI formatting + tests accordingly. - Reshape rate-limit models into per-category limits (
read/update/search/icon) and update CLI output + tests. - Add
smoke-test.shto exercise many read-only CLI commands; minor repo housekeeping updates.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/api/licence.rs | Makes licence response fields optional and adds coverage for omitted fields. |
| src/cmd/space/licence.rs | Formats optional licence fields safely; extends unit tests. |
| src/api/rate_limit.rs | Updates rate-limit response model to match nested category structure; updates tests. |
| src/cmd/rate_limit.rs | Prints rate-limit across categories; updates unit tests. |
| smoke-test.sh | Adds a script to run many read-only bl commands against a real space. |
| .gitignore | Ignores a smoke test result file. |
| .cspell/dicts/project.txt | Adds project-specific words used by the new script. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@smoke-test.sh`:
- Around line 16-21: Replace the hardcoded /tmp/bl_out with a secure temporary
file created via mktemp and ensure it is removed on script exit: create a temp
file variable (e.g., TMP_OUT=$(mktemp)) before running the command that uses
"$BL" and redirect stdout/stderr to that file, update the success/fail branches
that reference /tmp/bl_out to use the temp variable, and add a trap 'rm -f
"$TMP_OUT"' on EXIT to guarantee cleanup; apply the same change to the second
occurrence that prints or cats the temp output, and make sure PASS and label
usage remains unchanged.
- Around line 28-37: The run_capture block uses eval to assign command output
into a variable (eval "$varname"='$(cat /tmp/bl_out)') which allows command
injection; replace the eval usage by reading the output file into the named
variable safely with printf -v and avoid word-splitting/expansion (e.g., printf
-v "$varname" '%s' "$(cat /tmp/bl_out)") and similarly set the empty-case with
printf -v "$varname" '%s' '' instead of eval; also consider making the temp file
unique (e.g., include $$ or use mktemp) to avoid collisions when locating
/tmp/bl_out, and keep references to BL and SPACE_KEY intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b88e061c-df2d-4db1-aa6f-a72bb5bd42bc
📒 Files selected for processing (7)
.cspell/dicts/project.txt.gitignoresmoke-test.shsrc/api/licence.rssrc/api/rate_limit.rssrc/cmd/rate_limit.rssrc/cmd/space/licence.rs
Addresses review comment: use mktemp to avoid /tmp/bl_out clobbering/races, and printf -v instead of eval to avoid potential code execution from captured output.
- rate-limit: show all four categories (read/update/search/icon) in example - space licence: add note and example for plans where startDate/storageUsage are absent from the API response
There was a problem hiding this comment.
🧹 Nitpick comments (1)
smoke-test.sh (1)
41-41: Minor inconsistency: add'%s'format specifier for consistency.Line 36 uses
printf -v "$varname" '%s' "$output"with the format specifier, but line 41 omits it. While functionally equivalent for setting an empty string, adding the format specifier improves consistency.Suggested fix
- printf -v "$varname" '' + printf -v "$varname" '%s' ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@smoke-test.sh` at line 41, Replace the call that sets an empty value with printf -v "$varname" '%s' '' to match the earlier usage (printf -v "$varname" '%s' "$output"); update the statement using printf -v "$varname" '' so it includes the '%s' format specifier for consistency around the variable assignment of "$varname".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@smoke-test.sh`:
- Line 41: Replace the call that sets an empty value with printf -v "$varname"
'%s' '' to match the earlier usage (printf -v "$varname" '%s' "$output"); update
the statement using printf -v "$varname" '' so it includes the '%s' format
specifier for consistency around the variable assignment of "$varname".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 036b1824-a770-43b0-a784-e201fce891fb
📒 Files selected for processing (3)
smoke-test.shwebsite/docs/commands.mdwebsite/i18n/ja/docusaurus-plugin-content-docs/current/commands.md
✅ Files skipped from review due to trivial changes (2)
- website/docs/commands.md
- website/i18n/ja/docusaurus-plugin-content-docs/current/commands.md
23prime
deleted the
feature/129-130-fix-licence-rate-limit-deserialization
branch
Checklist
mainwebsite/docs/,website/i18n/ja/,README.md)Summary
bl space licencefailed withmissing field 'startDate'/missing field 'storageUsage'because the real API omits these fieldsbl rate-limitfailed withmissing field 'limit'because the struct expected a flatrateLimit.limitshape, but the real API returns four categories (read,update,search,icon)Reason for change
Struct definitions were written based on assumed API shapes rather than verified against the real Backlog API. Revealed by the new smoke-test script.
Changes
src/api/licence.rs: makestart_date,storage_limit,storage_usageOption; add test for response withoutstartDate/storageUsagesrc/cmd/space/licence.rs: handleOptionfields informat_licence_text; add testssrc/api/rate_limit.rs: introduceRateLimitCategory; restructureRateLimitInfowithread/update/search/iconfields; update testssrc/cmd/rate_limit.rs: updateformat_rate_limit_textto show all four categories; update testssmoke-test.sh: new smoke-test script.cspell/dicts/project.txt,.gitignore: housekeeping for spell-checkNotes
Closes #129
Closes #130