Code review · T7, real-pipeline corpus on CI runners¶
review-code on autopilot/T7 (weber8thomas/spectio#19), range origin/autopilot/integration...autopilot/T7, 2026-10-27, tick 8. Reviewers: code-reviewer and security-reviewer, independent, in parallel. Format: README.md.
| Round | Head | code-reviewer | security-reviewer |
|---|---|---|---|
| 1 | ea0f6bf |
changes (0 blockers, 1 major, 5 minors) | approve (0 blockers, 0 majors, 6 minors) |
| 2 | 7263253 |
approve (0 blockers, 0 majors, 2 minors) | (not re-run: round 1 approved; its fixes were checked by code-reviewer round 2) |
Round-1 fixes are in 1a14299. The round-2 minors are fixed in 0fe5196 (CR-7) and in the Progress page refresh (CR-8). code-simplifier ran on 0fe5196 and changed nothing: every candidate saved under a line or changed an error message. Test command:
python3 -m unittest tests/pipelines/real/test_run_corpus.py && python3 tests/pipelines/real/run_corpus.py --matrix pr
CODE-REVIEW verdict=approve blockers=0 majors=0 minors=2 simplify=0
SECURITY-REVIEW verdict=approve blockers=0 majors=0 minors=6
Findings¶
One line each. The reviewers' full evidence is in the session of 2026-10-27, tick 8.
| ID | Severity | Finding | Applied in |
|---|---|---|---|
| CR-1 | major | Free-disk, sampler and summary steps duplicated across the nfcore and snakemake jobs, and already drifting | 1a14299 (tests/pipelines/real/runner_probe.sh) |
| CR-2 | minor | Run-log tail skipped on timeout or cancel, and red when the log is missing | 1a14299 (failure() \|\| cancelled(), \|\| true) |
| CR-3 | minor | An interrupted clone by SHA left a broken checkout that local reruns reused | 1a14299 (cleanup and re-raise, unit test) |
| CR-4 | minor | rna-seq-star-deseq2 depends on the network and now gates ci-ok |
1a14299 (re-run rule in dev-env §10.2); tier choice assumed, inbox #45 |
| CR-5, CR-6 | minor | Research note undercounted the moves to manual; one external claim without a source |
1a14299 |
| CR-7 | minor | test_run_corpus.py never ran in CI |
0fe5196 (unit-test step in the matrix job) |
| CR-8 | minor | Progress page written before the review fixes | Progress page refresh |
| SEC-T7-02 | minor | clone() and actions/checkout accept branch names, so an entry could silently track a branch |
1a14299 (validate(): release tag or 40-hex SHA only) |
| SEC-T7-03 | minor | snakefile, args and directory reach an unquoted shell inside the composite action |
1a14299 (validate(): plain-token paths and args) |
| SEC-T7-05 | minor | setup-uv cache saved from a job that runs third-party code, with a key shared with required checks | 1a14299 (enable-cache: false) |
| SEC-T7-06 | minor | Destructive clean-up with no runner check | 1a14299 (if: runner.environment == 'github-hosted') |
| SEC-T7-01 | minor | Upstream repos pinned by mutable tags, and the corpus now gates ci-ok |
deferred: inbox #46 |
| SEC-T7-04 | minor | snakemake-github-action runs setup-miniconda@v3 and Miniforge latest, both unpinned |
deferred: inbox #46 |