mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 21:07:03 +01:00
Fix the 11 shellcheck findings that kept CI red since b83ce5d
"Pre-commit checks" has failed on every push since b83ce5d removed
shellcheck's --severity filter. The findings never appeared locally: CI
installs an older shellcheck than this machine, and 0.11.0 no longer emits
SC2002/SC2015/SC2119 at all. Reproduced by fetching shellcheck 0.9.0, which
matches CI's output exactly, and verified against both versions.
SC2015 x6 -- rewrote `A && B || C` as an explicit if. Each keeps its trailing
`|| true`, so a failing side effect still cannot abort the caller under set -e.
The first draft dropped it in run_phone.sh; a review pass caught that and it
was restored, since that call is the last statement of a function.
SC2119 x3 -- require_imagemagick and adb_select_device take an OPTIONAL first
argument, so the bare calls were already correct. Following shellcheck's
literal advice and passing "$@" would forward the script's own arguments as a
tool preference, which would be a real defect. Passing an explicit "" states
the same intent without changing behaviour, confirmed by running both forms.
SC2002 x2 -- `cat FILE | tr ...` becomes `tr ... < FILE`, keeping 2>/dev/null
so a missing file still yields an empty string.
No suppressions, no --severity filter, no exclusions added. Verified: 0
findings under both shellcheck versions, `pre-commit run shellcheck
--all-files` passes, test_adb_common 13/13, test_boot_repair 31/31, 1193
Python tests at 100% coverage.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ReVsw7qovYkkGbqcU9tn7
This commit is contained in:
parent
1b43ba093c
commit
9dd9aeb50e
@ -0,0 +1,21 @@
|
||||
{
|
||||
"title": "Fix the 11 shellcheck findings keeping CI red",
|
||||
"objective": "Restore the 'Pre-commit checks' GitHub Actions workflow to green. It has failed on every push since b83ce5d (2026-08-03), which removed shellcheck's --severity filter; CI runs an older shellcheck than the developer machine and reports 11 info/style findings that shellcheck 0.11.0 no longer emits. Fix each finding properly, with no behavioural change and no suppressions.",
|
||||
"acceptance_criteria": [
|
||||
"shellcheck 0.9.0, version-matched to CI, reports zero findings on all 9 edited files",
|
||||
"shellcheck 0.11.0, the developer machine version, also reports zero findings on those files",
|
||||
"pre-commit run shellcheck --all-files passes over the whole repository",
|
||||
"Every rewritten conditional keeps the trailing '|| true' the original had, so no guarded side effect can newly abort a caller under set -e",
|
||||
"require_imagemagick and adb_select_device behave identically to the bare calls they replace, confirmed by running both forms",
|
||||
"phone_focus_mode/lib/tests/test_adb_common.sh passes 13/13 and boot_recovery/tests/test_boot_repair.sh passes 31/31",
|
||||
"No shellcheck disable directive, no --severity filter, and no per-file exclusion is added"
|
||||
],
|
||||
"out_of_scope": [
|
||||
"Changing the behaviour of any edited script: every fix is semantics-preserving",
|
||||
"third_party/agent-skills/, which the shellcheck hook already excludes as vendored upstream code that would be lost on the next sync",
|
||||
"Pinning the shellcheck version in the CI workflow to remove the local/CI divergence at its root: considered, deferred as a separate change",
|
||||
"The mtk_root toolkit from commit 1b43ba0, which was already clean and is untouched here",
|
||||
"Device-level verification of curfew_enforcer.sh and hosts_enforcer.sh, which run on the phone under Magisk"
|
||||
],
|
||||
"verifier": "shellcheck 0.9.0 (CI-matched) and 0.11.0 over the edited files; pre-commit run shellcheck --all-files; phone_focus_mode/lib/tests/test_adb_common.sh; linux_configuration/scripts/boot_recovery/tests/test_boot_repair.sh; and the GitHub Actions 'Pre-commit checks' run on the pushed commit"
|
||||
}
|
||||
@ -0,0 +1,72 @@
|
||||
{
|
||||
"intent": "Turn the 'Pre-commit checks' CI workflow green again. It has failed on every push since b83ce5d (2026-08-03), the commit that removed shellcheck's --severity filter, because CI runs an older shellcheck than the developer machine and reports 11 findings that shellcheck 0.11.0 no longer emits.",
|
||||
"scope": [
|
||||
"linux_configuration/scripts/boot_recovery/boot-repair",
|
||||
"linux_configuration/scripts/diagnostics/line-speed-probe.sh",
|
||||
"linux_configuration/scripts/single_use/features/keepass_sync.sh",
|
||||
"linux_configuration/scripts/single_use/utils/{gif_to_square,txt_to_image}.sh",
|
||||
"phone_focus_mode/{curfew_enforcer,hosts_enforcer,run_phone}.sh",
|
||||
"phone_focus_mode/lib/tests/test_adb_common.sh",
|
||||
"Non-goal: changing any script's behaviour -- every fix is semantics-preserving",
|
||||
"Non-goal: suppressing findings via directives or a --severity filter",
|
||||
"Non-goal: third_party/, which the shellcheck hook already excludes as vendored code"
|
||||
],
|
||||
"changes": [
|
||||
"SC2015 x6: rewrote 'A && B || C' as an explicit if-statement in boot-repair, line-speed-probe.sh, keepass_sync.sh, curfew_enforcer.sh (x2) and run_phone.sh. Each retains its trailing '|| true' so a failing side effect still cannot abort the caller under set -e.",
|
||||
"SC2119 x3: require_imagemagick and adb_select_device both take an OPTIONAL first argument, so the bare calls were correct. Passing \"$@\" as shellcheck suggests would forward the script's own arguments as a tool preference, which would be a defect; passing an explicit \"\" states the same intent without changing behaviour.",
|
||||
"SC2002 x2: replaced 'cat FILE | tr ...' with 'tr ... < FILE' in hosts_enforcer.sh, keeping 2>/dev/null so a missing file still yields an empty string.",
|
||||
"Kept '|| true' on the _info call in run_phone.sh: it is the last statement of a function under set -e, so dropping it (as the first draft of this change did) would let a failed write to stderr abort the caller."
|
||||
],
|
||||
"verification": [
|
||||
{
|
||||
"command": "shellcheck-v0.9.0 -- <each of the 9 files> # version-matched to CI",
|
||||
"result": "pass",
|
||||
"evidence": "All 9 report OK. Before the change the same binary reproduced CI exactly: 6x SC2015, 3x SC2119, 2x SC2002. A full-repo scan under 0.9.0 leaves findings only in third_party/agent-skills/, which the hook excludes via '^third_party/'."
|
||||
},
|
||||
{
|
||||
"command": "pre-commit run shellcheck --all-files",
|
||||
"result": "pass",
|
||||
"evidence": "shellcheck...Passed over the whole repository."
|
||||
},
|
||||
{
|
||||
"command": "bash phone_focus_mode/lib/tests/test_adb_common.sh",
|
||||
"result": "pass",
|
||||
"evidence": "Results: 13 passed, 0 failed."
|
||||
},
|
||||
{
|
||||
"command": "bash linux_configuration/scripts/boot_recovery/tests/test_boot_repair.sh",
|
||||
"result": "pass",
|
||||
"evidence": "31 passed, 0 failed."
|
||||
},
|
||||
{
|
||||
"command": "behavioural equivalence check for require_imagemagick",
|
||||
"result": "pass",
|
||||
"evidence": "Sourcing lib/common.sh and calling both forms: require_imagemagick \"\" -> OK, MAGICK_CMD=magick; bare require_imagemagick -> OK, MAGICK_CMD=magick. Identical result."
|
||||
},
|
||||
{
|
||||
"command": "behavioural equivalence check for the useless-cat rewrite",
|
||||
"result": "pass",
|
||||
"evidence": "Against a file containing ' 1 ': old 'cat F | tr -d [:space:]' -> [1]; new 'tr -d [:space:] < F' -> [1]. Against a nonexistent path the new form still yields [] with the error suppressed."
|
||||
},
|
||||
{
|
||||
"command": "bash -n on all 8 edited executable scripts",
|
||||
"result": "pass",
|
||||
"evidence": "All report syntax OK."
|
||||
},
|
||||
{
|
||||
"command": "bash ~/.claude/scripts/finish.sh gate ~/testsAndMisc <9 files>",
|
||||
"result": "pass",
|
||||
"evidence": "FINISH_GATE {\"risk\":\"review\",\"changed_files\":9,\"phase\":\"ok\"} with 1193 Python tests passing at 100% coverage. The review pass flagged the dropped '|| true' in run_phone.sh, which was then restored."
|
||||
}
|
||||
],
|
||||
"risks": [
|
||||
"The underlying local/CI divergence remains: CI installs a different shellcheck version than the developer machine, so a future finding can again pass locally and fail in CI. Pinning the version in the workflow would remove the class of problem; that was considered and deferred as a separate change.",
|
||||
"These files were not otherwise exercised end-to-end. curfew_enforcer.sh and hosts_enforcer.sh run on the phone under Magisk and were verified by static analysis, syntax check and unit tests rather than a device run.",
|
||||
"keepass_sync.sh's case arm is now a single-line if-then-fi, which is denser to read than the original; behaviour is unchanged."
|
||||
],
|
||||
"rollback": [
|
||||
"git revert the commit; each change is confined to one or two lines per file and no interfaces changed.",
|
||||
"After reverting, expect 'Pre-commit checks' CI to go red again on the same 11 findings while local shellcheck 0.11.0 continues to report none.",
|
||||
"Re-run pre-commit run shellcheck --all-files plus the phone_focus_mode and boot_recovery test suites to confirm the revert left both in their prior state."
|
||||
]
|
||||
}
|
||||
@ -363,7 +363,9 @@ check_fstab_options() {
|
||||
' OFS='\t' "$fstab" > "${fstab}.new"
|
||||
mv "${fstab}.new" "$fstab"
|
||||
repaired "fstab /boot options reset to 'defaults'"
|
||||
is_live && systemctl daemon-reload 2> /dev/null || true
|
||||
if is_live; then
|
||||
systemctl daemon-reload 2> /dev/null || true
|
||||
fi
|
||||
return 0
|
||||
}
|
||||
|
||||
|
||||
@ -80,7 +80,9 @@ verdict_for() {
|
||||
cleanup() {
|
||||
local p
|
||||
for p in "${PIDS[@]:-}"; do
|
||||
[[ -n "$p" ]] && kill "$p" 2>/dev/null || true
|
||||
if [[ -n "$p" ]]; then
|
||||
kill "$p" 2>/dev/null || true
|
||||
fi
|
||||
done
|
||||
wait 2>/dev/null || true
|
||||
}
|
||||
|
||||
@ -88,7 +88,7 @@ remote_get() {
|
||||
}
|
||||
remote_hash() {
|
||||
case "$REMOTE_MODE" in
|
||||
file) [[ -f "$REMOTE_FILE" ]] && sha "$REMOTE_FILE" || true ;;
|
||||
file) if [[ -f "$REMOTE_FILE" ]]; then sha "$REMOTE_FILE" || true; fi ;;
|
||||
webdav) curl -fsS -u "$DUFS_USER:$DUFS_PASS" "${REMOTE_URL}?hash" 2>/dev/null || true ;;
|
||||
esac
|
||||
}
|
||||
|
||||
@ -49,7 +49,9 @@ if ! command -v magick &> /dev/null && ! command -v convert &> /dev/null; then
|
||||
fi
|
||||
fi
|
||||
|
||||
require_imagemagick || exit 1
|
||||
# Empty arg = no preference between magick/convert. Passing "$@" here would
|
||||
# forward this script's own arguments as a tool preference, which is wrong.
|
||||
require_imagemagick "" || exit 1
|
||||
|
||||
# Set up identify command (IM7: magick identify, IM6: identify)
|
||||
if [[ ${MAGICK_CMD} == "magick" ]]; then
|
||||
|
||||
@ -35,7 +35,9 @@ EOF
|
||||
}
|
||||
|
||||
# Check if ImageMagick is installed and determine which command to use
|
||||
require_imagemagick || exit 1
|
||||
# Empty arg = no preference between magick/convert. Passing "$@" here would
|
||||
# forward this script's own arguments as a tool preference, which is wrong.
|
||||
require_imagemagick "" || exit 1
|
||||
|
||||
# Parse arguments
|
||||
if [[ $# -lt 1 ]]; then
|
||||
|
||||
@ -137,7 +137,9 @@ restore_grayscale() {
|
||||
case "$en" in ''|null) en=0 ;; esac
|
||||
case "$lv" in ''|null) lv=-1 ;; esac
|
||||
settings put secure accessibility_display_daltonizer_enabled "$en" 2>/dev/null || true
|
||||
[ "$lv" != "-1" ] && settings put secure accessibility_display_daltonizer "$lv" 2>/dev/null || true
|
||||
if [ "$lv" != "-1" ]; then
|
||||
settings put secure accessibility_display_daltonizer "$lv" 2>/dev/null || true
|
||||
fi
|
||||
}
|
||||
|
||||
# ---- Layer 2: Do-Not-Disturb (alarms only) ----
|
||||
@ -232,8 +234,9 @@ NET_BUILT=""
|
||||
# Refresh the cached UID list (one `pm list packages -U` fork). Called once per
|
||||
# main tick so the fast watchdog can rebuild from the cache without forking.
|
||||
refresh_uid_cache() {
|
||||
night_uids > "$CURFEW_NET_UID_CACHE.tmp" 2>/dev/null \
|
||||
&& mv "$CURFEW_NET_UID_CACHE.tmp" "$CURFEW_NET_UID_CACHE" 2>/dev/null || true
|
||||
if night_uids > "$CURFEW_NET_UID_CACHE.tmp" 2>/dev/null; then
|
||||
mv "$CURFEW_NET_UID_CACHE.tmp" "$CURFEW_NET_UID_CACHE" 2>/dev/null || true
|
||||
fi
|
||||
}
|
||||
|
||||
# Rebuild the chain from cache for whichever iptables variants exist. No pm fork.
|
||||
|
||||
@ -55,7 +55,7 @@ acquire_lock() {
|
||||
old_pid="$(cat "$PIDFILE")"
|
||||
if kill -0 "$old_pid" 2>/dev/null; then
|
||||
local cmdline
|
||||
cmdline="$(cat "/proc/$old_pid/cmdline" 2>/dev/null | tr '\0' ' ')"
|
||||
cmdline="$(tr '\0' ' ' < "/proc/$old_pid/cmdline" 2>/dev/null)"
|
||||
if echo "$cmdline" | grep -q "hosts_enforcer"; then
|
||||
echo "hosts_enforcer already running (PID $old_pid)"
|
||||
exit 0
|
||||
@ -82,7 +82,7 @@ sha256_of() {
|
||||
workout_active() {
|
||||
[ -f "$WORKOUT_ACTIVE_FILE" ] || return 1
|
||||
local v
|
||||
v="$(cat "$WORKOUT_ACTIVE_FILE" 2>/dev/null | tr -d '[:space:]')"
|
||||
v="$(tr -d '[:space:]' < "$WORKOUT_ACTIVE_FILE" 2>/dev/null)"
|
||||
[ "$v" = "1" ]
|
||||
}
|
||||
|
||||
|
||||
@ -172,7 +172,8 @@ TRUSTED_MODEL='Pixel 7rm -rf /dangerline2'
|
||||
TRUSTED_FINGERPRINT='google/pixel:14/UP1A.231005.007/evilcmd'
|
||||
EOF
|
||||
|
||||
if (adb_select_device >/dev/null 2>&1); then
|
||||
# Empty arg = no requested serial, which is what this case exercises.
|
||||
if (adb_select_device "" >/dev/null 2>&1); then
|
||||
return 1
|
||||
fi
|
||||
|
||||
|
||||
@ -68,7 +68,12 @@ _auto_repair_minor_drift() {
|
||||
fi
|
||||
fi
|
||||
|
||||
[[ ${repaired} -gt 0 ]] && _info "Minor repairs applied: ${repaired}" || true
|
||||
# `|| true` retained from the original `&& ... || true`: this is the last
|
||||
# statement of the function under `set -e`, so a failed write to stderr
|
||||
# would otherwise abort the caller.
|
||||
if [[ ${repaired} -gt 0 ]]; then
|
||||
_info "Minor repairs applied: ${repaired}" || true
|
||||
fi
|
||||
}
|
||||
|
||||
cmd_auto() {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user