aws-samples / aws-samples/aws-transform-custom-samples
scaled-execution-bash: parallel runs crash on macOS, and failed transformations are reported as SUCCESS
- Dominant language
- Shell
- Stars
- 37
- Forks
- 18
- Avg merge
- 5d 9h
- Merged PRs (30d)
- 7
Description
Six bugs in `scaled-execution-bash/atx-custom-automation.sh`. Two of them stop a default-configuration run outright on macOS; the other four mean the results the script reports cannot be trusted.
I have a fix for all six, tested end to end. Opening this first per CONTRIBUTING rather than sending the PR straight away.
## 1. Parallel runs die at the default settings on macOS (`new_pids[@]: unbound variable`)
Line 966-968:
```bash
local new_pids=()
for pid in "${pids[@]}"; do
kill -0 "$pid" 2>/dev/null && new_pids+=($pid)
done
pids=("${new_pids[@]}")
```
Under `set -u`, bash 3.2 - which is `/bin/bash` on macOS, and the shebang here is `#!/bin/bash` - treats an empty-array expansion as unbound and aborts. It fires as soon as every in-flight job finishes while repos are still queued, so any run where repo count > `--max-jobs` can die. With 40 repos at the default `--max-jobs 10`, only 10 were processed.
The wait loop 40 lines below already has the right guard (`"${new_pids[@]+"${new_pids[@]}"}"` at line 1011) - this copy just missed it.
## 2. `--resume` crashes once everything is COMPLETED (`filtered_repos[@]: unbound variable`)
Same empty-array class, line 1902. `log_success "All repositories already completed!"` at line 1911 is unreachable. This is mostly hidden today because bug 4 leaves repos stuck in a non-COMPLETED state, so "everything completed" rarely happens.
## 3. Failed transformations are recorded as SUCCESS
Lines 595/598 append `|| true` to the atx pipeline, then line 600 reads `${PIPESTATUS[0]}`:
```bash
"${atx_cmd_args[@]}" 2>&1 | tee -a "$log_file" || true
cmd_exit=${PIPESTATUS[0]}
```
`a | b || true` is one AND-OR list. When the pipeline fails, bash runs `true`, and that resets `PIPESTATUS` to `(0)`. So `cmd_exit` is 0 exactly when atx failed:
```
$ bash -c 'set -euo pipefail; (exit 7) | cat || true; echo ${PIPESTATUS[0]}'
0
```
The only remaining detector is the grep over the nine hardcoded `ATX_ERROR_PATTERNS`, so any failure whose output does not contain one of those strings - a build failure, a crash, an OOM, a timeout - is written to results.txt as SUCCESS. With a stub `atx` that exits 1, a 3-repo run reported `Success: 3 | Failed: 0`.
## 4. Status updates are dropped under contention, which corrupts `--resume`
Line 182:
```bash
mkdir "$lockfile" 2>/dev/null || return 0
```
One attempt, and on failure it returns success without writing anything. `write_with_lock` 40 lines above already retries for 30s - `mark_repo_status` just does not use it.
Measured on 40-repo parallel runs: repos that finished (they have a row in results.txt) but whose `.atx-batch-status` never reached a terminal state - 7, 7 and 11 across three runs. Those repos get re-run on the next `--resume`.
## 5. `--resume` wipes the previous attempt's results
Line 1946 writes the results.txt header unconditionally, and in resume mode `OUTPUT_DIR` resolves to the previous run's directory. A 40-row results.txt became 7 rows after a resume, and the summary then reported `Total Repositories | 7 ... Success Rate | 100%`.
## 6. The summary report aborts before printing (only visible once 3 is fixed)
Line 1275 ends a `while read` body with `[[ "$status" == "FAILED" ]] && printf ...`. When the last row is SUCCESS the AND-list returns 1, the `while` returns 1, and `set -e` kills main() part-way through the summary block - console summary, failed_repos.csv and the closing message never run, and the script exits 1.
This is pre-existing but dormant, because bug 3 means `failed` is almost always 0. Fixing 3 makes it fire on nearly every batch that has a failure, so the two cannot be separated.
## What the fix looks like
Six commits, ~104 lines, one file:
- guard both empty-array expansions the way line 1011 already does
- `set +o pipefail` around the atx pipeline instead of `|| true`, capturing `PIPESTATUS[0]` inside each branch
- give `mark_repo_status` the same retry loop as `write_with_lock`, failing **open** on timeout (returning non-zero there kills the caller under `set -euo pipefail` - I tried it)
- write the results.txt header only when the file does not exist, and drop the rows for repos about to be re-run
- make the FAILED loop return 0
Verified with a stub `atx` binary and local git repos, so no AWS calls and no ATX service access needed:
| | before | after |
|---|---|---|
| 40-repo parallel, lost status rows | 7 / 7 / 11 | 0 / 0 / 0 |
| stub atx exits 1 | `SUCCESS` | `FAILED` |
| resume ladder (40 repos, 10 failing) | results.txt truncated to 7 rows | 40 rows, 30 -> 37 -> 40 SUCCESS, then "All repositories already completed!" |
Happy to send it as one PR, or split if you would rather review the two crashes separately from the reporting fixes. Also happy to leave it if this script is not something you want changes to right now.
Contributor guide
Research direction
Start in scaled-execution-bash/atx-custom-automation.sh, focusing on the cited parallel-job, resume, status-lock, pipeline, and summary-report sections. Reproduce the cases with a stub atx binary and local git repositories; done means parallel runs retain all statuses, failures are reported as FAILED, resume preserves results, completed runs exit cleanly, and summaries finish.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- bash
- Domain
- cli, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100