Support multi-character paste for headless input_otp and remove maxlength on slots#11
Support multi-character paste for headless input_otp and remove maxlength on slots#11
maxlength on slots#11Conversation
📝 WalkthroughWalkthroughRefactors OTP input to support multi-character pastes distributed across slots via a new public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/ci/retry.sh`:
- Around line 27-38: The retry loop in retry.sh currently always exits 1 on
exhaustion, losing the wrapped command’s real exit code; modify the loop to
capture the wrapped command’s exit status (e.g., save "$?" into a variable like
last_rc each time the command fails) and after the loop ends use exit $last_rc
instead of exit 1 so callers see the original command’s failure code; keep the
existing success path (exit 0) and sleep behavior using the existing variables
(command, attempt, attempts, sleep_seconds).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f14aaf74-92ab-4ab0-b7e5-21a441bc3d52
📒 Files selected for processing (3)
.github/workflows/test.ymlscripts/check.shscripts/ci/retry.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🔇 Additional comments (3)
scripts/ci/retry.sh (1)
5-23: Good guardrails on the retry contract.The arity and integer validation here will fail fast on misconfigured CI calls instead of producing harder-to-debug shell errors later.
.github/workflows/test.yml (1)
63-65: Nice CI hardening at the network boundary.Retrying
gleam deps downloadhere should reduce transient Hex/download failures without broadening retries to the rest of the job.scripts/check.sh (1)
9-9: Good resilience improvement for the shared check path.Wrapping dependency download here hardens the most failure-prone external call before the rest of the checks execute.
| for attempt in $(seq 1 "$attempts"); do | ||
| echo "Attempt ${attempt}/${attempts}: ${command[*]}" >&2 | ||
| if "${command[@]}"; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [ "$attempt" -lt "$attempts" ]; then | ||
| sleep "$sleep_seconds" | ||
| fi | ||
| done | ||
|
|
||
| exit 1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Preserve the wrapped command’s final exit code.
After the last failed attempt this always exits 1, so the original failure mode is lost. That makes CI diagnostics worse and prevents callers from distinguishing retry exhaustion from command-specific failures.
💡 Proposed fix
+status=0
for attempt in $(seq 1 "$attempts"); do
echo "Attempt ${attempt}/${attempts}: ${command[*]}" >&2
if "${command[@]}"; then
exit 0
+ else
+ status=$?
fi
if [ "$attempt" -lt "$attempts" ]; then
sleep "$sleep_seconds"
fi
done
-exit 1
+exit "$status"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for attempt in $(seq 1 "$attempts"); do | |
| echo "Attempt ${attempt}/${attempts}: ${command[*]}" >&2 | |
| if "${command[@]}"; then | |
| exit 0 | |
| fi | |
| if [ "$attempt" -lt "$attempts" ]; then | |
| sleep "$sleep_seconds" | |
| fi | |
| done | |
| exit 1 | |
| status=0 | |
| for attempt in $(seq 1 "$attempts"); do | |
| echo "Attempt ${attempt}/${attempts}: ${command[*]}" >&2 | |
| if "${command[@]}"; then | |
| exit 0 | |
| else | |
| status=$? | |
| fi | |
| if [ "$attempt" -lt "$attempts" ]; then | |
| sleep "$sleep_seconds" | |
| fi | |
| done | |
| exit "$status" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ci/retry.sh` around lines 27 - 38, The retry loop in retry.sh
currently always exits 1 on exhaustion, losing the wrapped command’s real exit
code; modify the loop to capture the wrapped command’s exit status (e.g., save
"$?" into a variable like last_rc each time the command fails) and after the
loop ends use exit $last_rc instead of exit 1 so callers see the original
command’s failure code; keep the existing success path (exit 0) and sleep
behavior using the existing variables (command, attempt, attempts,
sleep_seconds).
1 participant
Motivation
on_inputhandlers to apply multi-character updates (paste) across subsequent OTP slots instead of only replacing the single slot.maxlength="1"attribute from rendered slot inputs so browser-level truncation doesn't interfere with controlled multi-character handling.Description
replace_slots_from_index/3andinput_otp_apply_slot_input/3to compute resulting slot string when an input contains multiple graphemes and fan pasted values across following slots.on_inputhandlers with a call toinput_otp_apply_slot_input(chars, index, input)so both single-character and multi-character inputs are handled correctly.maxlength="1"attribute frominput_otp_slotrequired attributes so controlled logic can accept and distribute multi-character input.first_grapheme_or_empty.test/input_otp_test.gleamto assert the new paste behavior and thatmaxlengthis not present.Testing
gleam test, and the updatedinput_otptests covering headless mutators, rendering, and multi-character paste behavior passed.Codex Task