Fix Fail-Slow / exit-code masking#59
Merged
wpak-ai merged 6 commits intoJul 16, 2026
Merged
Conversation
|
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:
📝 WalkthroughWalkthroughThe change centralizes repository-dispatch handling, strengthens translation failure propagation, adds Weblate-aware exit-code composition, consolidates lint binary caching, updates Git configuration, and refreshes related documentation and tests. ChangesDispatch and workflow orchestration
Tooling and documentation
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (2)
tests/test_submodule_ops.bats (1)
128-157: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for fatal finalization status 2.
The changed submodule path can provide
finalize_rc=2, but these tests use3and never define whether status2should be collapsed or propagated. Add an explicitcombine_batch_and_finalize_rc 2 0assertion matching the documented contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_submodule_ops.bats` around lines 128 - 157, Add a test for combine_batch_and_finalize_rc with submodule_fatal set to 2, finalize_rc set to 2, and weblate_rc set to 0; assert the returned status matches the documented contract for fatal finalization status 2.tests/test_lib.bats (1)
200-212: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPin the fatal return contract in both tests.
Both tests currently accept any non-zero status, so they would miss regressions that return a different code.
tests/test_lib.bats#L200-L212: assertclone_reporeturns exactly2.tests/test_translations_submodule.bats#L103-L127: assert exactly2and verify later submodules are not processed after the first fatal update.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_lib.bats` around lines 200 - 212, Update the failure assertion in tests/test_lib.bats lines 200-212 to require clone_repo returns exactly status 2. In tests/test_translations_submodule.bats lines 103-127, assert the fatal update returns exactly 2 and verify that subsequent submodules are not processed after the first fatal update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ARCHITECTURE.md`:
- Around line 207-220: Update the start-local entry in the “Workflow job exit”
table to use the actual finalization variable, rc, in the
combine_batch_and_finalize_rc invocation. Keep the documented weblate_rc
argument and the existing description of the three-source collapse logic
unchanged.
---
Nitpick comments:
In `@tests/test_lib.bats`:
- Around line 200-212: Update the failure assertion in tests/test_lib.bats lines
200-212 to require clone_repo returns exactly status 2. In
tests/test_translations_submodule.bats lines 103-127, assert the fatal update
returns exactly 2 and verify that subsequent submodules are not processed after
the first fatal update.
In `@tests/test_submodule_ops.bats`:
- Around line 128-157: Add a test for combine_batch_and_finalize_rc with
submodule_fatal set to 2, finalize_rc set to 2, and weblate_rc set to 0; assert
the returned status matches the documented contract for fatal finalization
status 2.
🪄 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: e659084a-c016-478f-b46e-844bd8cfdaa3
📒 Files selected for processing (7)
.github/workflows/assets/lib.sh.github/workflows/assets/submodule_ops.sh.github/workflows/start-translation.ymldocs/ARCHITECTURE.mdtests/test_lib.batstests/test_submodule_ops.batstests/test_translations_submodule.bats
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ARCHITECTURE.md`:
- Around line 168-177: Update the architecture diagram’s finalization flow
around FTL and CB so finalization status routes directly into the combiner,
independently of the Weblate path. Preserve the existing FTL-to-TW-to-CB flow
for successful finalization while showing that CB can also receive FTL outcomes
when Weblate is skipped after failure.
In `@scripts/lint.sh`:
- Around line 35-53: Update the extraction flow guarded by the extract_dir check
to validate that the expected inner binary exists, rather than treating any
existing directory as complete. Ensure failed gzip and xz extractions remove the
partial extract_dir before exiting, so subsequent invocations reverify and retry
extraction; keep the final copy in place only after successful validation.
In `@scripts/trigger-dispatch-common.sh`:
- Around line 31-39: Update the repository resolution flow around
infer_repo_from_git so it attempts git-origin inference before checking
DEFAULT_REPO. Preserve the existing GITHUB_REPOSITORY override first, and retain
DEFAULT_REPO only as the fallback when inference produces no repository.
🪄 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: a6c9ed22-1825-4ad0-8b4d-ebf5979b40fe
📒 Files selected for processing (19)
.github/workflows/assets/README.md.github/workflows/assets/add_submodules.sh.github/workflows/assets/create-tag.yml.github/workflows/assets/lib.sh.github/workflows/assets/submodule_ops.sh.github/workflows/assets/translation.sh.github/workflows/start-translation.yml.github/workflows/sync-translation.ymlCHANGELOG.mdREADME.mddocs/ARCHITECTURE.mddocs/GETTING-STARTED.mddocs/endpoint-contract.mdscripts/install-git-hooks.shscripts/lint.shscripts/trigger-add-submodules.shscripts/trigger-dispatch-common.shscripts/trigger-start-translation.shtests/helpers/common.bash
💤 Files with no reviewable changes (3)
- .github/workflows/assets/add_submodules.sh
- .github/workflows/assets/translation.sh
- .github/workflows/assets/lib.sh
Collaborator
Author
|
@coderabbitai, review |
|
✅ Action performedReview finished.
|
henry0816191
approved these changes
Jul 16, 2026
henry0816191
left a comment
henry0816191
left a comment
There was a problem hiding this comment.
In CHANGELOG.md, add a ### Fixed (or ### Changed) bullet under [Unreleased] summarizing fail-fast clone/finalize and the three-source exit-code combiner.
wpak-ai
approved these changes
Jul 16, 2026
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
3 participants
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.
Close #56.
Summary by CodeRabbit
repository_dispatchwrapper used by trigger scripts.