Skip to content

qa: fix bash-5.3 unbound-variable crash in the DM transient-retry path#1661

Merged
100yenadmin merged 1 commit into
mainfrom
retry-ellipsis-fix
Jul 22, 2026
Merged

qa: fix bash-5.3 unbound-variable crash in the DM transient-retry path#1661
100yenadmin merged 1 commit into
mainfrom
retry-ellipsis-fix

Conversation

@100yenadmin

Copy link
Copy Markdown
Member

adv_live3's beat-2 DM timeout (rc=124) was correctly classified TRANSIENT, but the retry-announce echo — retry $((attempt+1))/$max… — crashed with max�: unbound variable before the retry ran, turning a recoverable timeout into a fatal beat and ending the run.

Root cause: homebrew bash 5.3.15 under set -u gloms the first byte of the U+2026 ellipsis into the variable-name scan when it directly abuts $max (/bin/bash 3.2 parses it fine). Red-first repro in the commit message. Fix: brace the expansion (${max}…). Repo-wide sweep for $var abutting a multibyte char found exactly this one instance.

This unblocks the #1656/#1657/#1659 acceptance rerun (adv_live4).

…under set -u gloms the multibyte char's first byte into the varname ('max?: unbound variable'), killing the transient-retry path the first time it ever ran (adv_live3 beat-2 rc=124 timeout became a fatal beat instead of a fresh-session retry; repro: bash5.3 -c 'set -u; max=4; echo "$max?"')
@100yenadmin
100yenadmin enabled auto-merge (squash) July 22, 2026 08:06
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9feed5d0-c90d-43e4-b1c6-9683f7581936

📥 Commits

Reviewing files that changed from the base of the PR and between fcc5408 and bf4c572.

📒 Files selected for processing (1)
  • qa/run_adventure.sh
📜 Recent review details
⏰ Context from checks skipped due to timeout. (14)
  • GitHub Check: paint-drift-gate
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: qa-release-gate-tests
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (python)
  • GitHub Check: viewer-tests
  • GitHub Check: test
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (csharp)
  • GitHub Check: qa-release-gate-tests
  • GitHub Check: paint-drift-gate
  • GitHub Check: viewer-tests
  • GitHub Check: test
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Treat /Users/lume/WorldOS as the canonical local Mac app/private-art checkout for WorldOS GUI and native-app testing.
Use /Volumes/LEXAR/Codex for Codex artifacts, scratch files, screenshots, reports, and downloaded CI/VM artifacts.
Before install, build, or test commands, verify pwd; GUI/native app runs must use /Users/lume/WorldOS or an approved same-disk worktree configuration.
Use the codex/ prefix for new branches unless instructed otherwise.

Files:

  • qa/run_adventure.sh
🔇 Additional comments (1)
qa/run_adventure.sh (1)

382-382: LGTM!


📝 Walkthrough

Priority Level: P1

The ${max} expansion fix correctly prevents Bash 5.3 set -u from interpreting the trailing ellipsis as part of the variable name, allowing transient timeout retries to proceed instead of terminating the run. No additional merge-blocking issues identified.

Walkthrough

turn_retry() updates the formatting of its transient empty-output retry message to display ${max} differently. Retry control flow is unchanged.

Changes

Retry logging

Layer / File(s) Summary
Update retry message formatting
qa/run_adventure.sh
The empty-output retry log uses ${max} in the displayed retry counter with minor text formatting changes; retry behavior is unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the bug well, but it does not follow the required PR template or include most mandatory sections. Rewrite the PR description using the template headings and fill in at least Summary, Linked Issue, Changes, Validation, rollout, and evidence sections.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main fix: a Bash 5.3 unbound-variable crash in the DM transient-retry path.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch retry-ellipsis-fix

Comment @coderabbitai help to get the list of available commands.

@evaos-code-review-bot

evaos-code-review-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown

evaOS review status: completed

PR: #1661 - qa: fix bash-5.3 unbound-variable crash in the DM transient-retry path
Head: bf4c572f35eb83b638324b2021c7f972566a6e55
Updated: 2026-07-22T08:12:17.456Z

evaOS review completed for this PR head.

Automation note: agents should wait for this comment to reach completed, stale_head, closed_or_merged_before_review, skipped, or failed before treating evaOS review as settled for this head. provider_deferred means evaOS still intends to retry.

PR URL: #1661

Review URL: #1661 (review)

@evaos-code-review-bot evaos-code-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Walkthrough

PR: #1661 - qa: fix bash-5.3 unbound-variable crash in the DM transient-retry path
Head: bf4c572f35eb83b638324b2021c7f972566a6e55 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).

Estimated review effort: 1/5 (~12 min)

Changed Files

File Status Churn Purpose Risk
qa/run_adventure.sh modified +1/-1 Changed file Low

Review Signal

No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.

Risk Taxonomy

No finding categories.

Validation and Proof

1 required validation/proof recommendation(s) selected from changed files.

  • required: Unity editor or Play Mode smoke - WorldOS repo profile implies Unity runtime risk. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
    Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
    Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
    Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.

Related Context

Related issues/PRs: #1656, #1657, #1659.
Suggested labels: none.
Suggested reviewers: none from current metadata.

Review Settings Preview

  • Profile: assertive
  • Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
  • Path instructions: Assets/** - Prioritize scene, prefab, save-state, asset-reference, and gameplay regressions.
  • Path instructions: ProjectSettings/** - Treat build, platform, input, graphics, and release behavior changes as high risk.
  • Label suggestions: unity, gameplay, regression-hardening
  • Reviewer suggestions: none
  • Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
  • Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks

Pre-merge checklist

  • Inline comments target current RIGHT-side diff lines.
  • No secret-like content survived into posted inline comments.
  • REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
  • Required behavior proof is present or not applicable.
  • Labels and reviewers are suggestions only; the bot did not auto-apply them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant