mirror of https://github.com/azaion/satellite-provider.git synced 2026-06-21 22:41:14 +00:00

Files

T

Oleksandr Bezdieniezhnykh ab437a15df [AZ-504] Fix grep | wc -l pipefail crash in PT-08 batch counting

scripts/run-performance-tests.sh:416-417 used `grep -o ... | wc -l`
to count `"status":"accepted"` and `"status":"rejected"` markers in
the PT-08 batch response. On the happy path (rejected=0) grep -o
exits 1, and under `set -o pipefail` + `set -e` (line 16) the
pipeline killed the script before reaching any of PT-08's reporting
code — reproducing twice in the cycle-3 perf-harness leftover
(replay #2 + #3 post-AZ-500).

Fix: neutralise grep's no-match exit locally with `|| true` on the
grep stage of each pipeline. `grep -o | wc -l` is kept (not swapped
for `grep -c`) because the PT-08 response is compact JSON — all
items live on one line, so `grep -c` would always return 1 and lose
occurrence-count semantics. An 8-line comment explains why grep
cannot fail for I/O at this code path (file is curl-written, HTTP
200 gated).

AC-1 + AC-2 verified in-place against a standalone harness under
`set -e -o pipefail` (compact-JSON, mixed-status, edge-empty
cases). AC-3 + AC-4 are Step 15 (Performance Test) obligations by
spec design — the leftover deletion (AC-4) is "in the same commit"
as the green full perf run.

Batch report: _docs/03_implementation/batch_01_cycle5_report.md.
Code review: _docs/03_implementation/reviews/batch_01_cycle5_review.md
— PASS, no findings.

Co-authored-by: Cursor <cursoragent@cursor.com>

2026-05-12 16:32:36 +03:00

2.3 KiB

Raw Blame History

Code Review Report

Batch: 01 (cycle 5) — AZ-504 (1 SP) Date: 2026-05-12 Verdict: PASS

Phase Summary

Phase	Result
1. Context Loading	OK — task spec read; intent = tolerate grep exit 1 on no-match while preserving occurrence-count semantics; do NOT change PT-08 threshold or production code.
2. Spec Compliance	OK — AC-1 + AC-2 verified in standalone harness under `set -e -o pipefail`. AC-3 + AC-4 staged by spec design (AC-4 GIVEN clause couples them to the Step 15 full perf run). No Spec-Gap.
3. Code Quality	OK — `\|\| true` is locally scoped, accompanied by an 8-line comment explaining why grep cannot fail for I/O at this code path; coderule.mdc allows this with comment. Mirror-image accepted/rejected lines are intentional, not duplication worth extracting.
4. Security Quick-Scan	OK — no SQL, no command injection, no secrets, no input validation touch.
5. Performance Scan	OK — same `grep -o \| wc -l` complexity as before; `\|\| true` adds zero overhead on the happy path.
6. Cross-Task Consistency	N/A — single-task batch.
7. Architecture Compliance	N/A — shell script, not in any C# component; no layer / Public API / cycle / duplicate-symbol concerns.

Findings

None.

Notes

Risk 1 (compact JSON vs grep -c) was verified empirically against SatelliteProvider.Common/DTO/UavTileBatchUploadResponse.cs — the ASP.NET Core default serializer emits the full Items list on a single line. grep -c would have collapsed all occurrences to a line count of 1. The chosen fix (grep -o ... \|\| true \| wc -l) preserves occurrence semantics. The 8-line in-script comment captures this so a future maintainer doesn't re-introduce grep -c.
Risk 2 (other vulnerable grep ... \| wc -l sites) — Grep pass over the whole script found only the two sites named in the spec (416, 417). The third grep -o site (line 141, region status polling) is protected by head -1 \|\| true. No other defensive work required.
Test infra note — no BATS/shell-test framework in this project. Adding one for a 1-SP fix is infra creep. The standalone harness in the batch report is the established equivalent verification.

Verdict

PASS — no findings of any severity. Proceed to commit. AC-3 + AC-4 are Step 15 obligations, not Step 10 gaps.

2.3 KiB Raw Blame History