phone_focus_mode: split adb_common.sh 346 -> 155 + two libs

adb_trusted.sh (136)  the enrolled-device record: save, list, forget, match,
                        and refuse to act on an unenrolled phone
  adb_locking.sh (76)   the single-instance run lock and the cooldown markers

Both are sourced at the end of adb_common.sh from its own directory, so every
consumer keeps getting the whole API from the one source line it already has.
No consumer was edited. Nothing deploys a copy of this lib elsewhere, so the
deployed-copy blocker that stops install_leechblock.sh does not apply.

Picked the seam by coverage, not size: 13 of 23 functions are named by
test_adb_common.sh, and both extracted groups sit inside that covered set.

One real edit. _matches_trusted_record used to shadow TRUSTED_DEVICE_FILE with
a `local` so _load_trusted_device_values would read it; across a file boundary
shellcheck can't see that and reported SC2034. Fixed by making the function
take the path as an argument, which is what the caller meant -- not by a
suppression. shellcheck passes with none added, at a hook that runs with no
severity filter.

16/16 tests still pass, and sourcing adb_common.sh alone still defines every
function from all three files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
This commit is contained in:
Krzysztof Rudnicki 2026-08-17 10:31:34 +02:00
parent 4dcc0beee1
commit 466b074dd7
4 changed files with 281 additions and 200 deletions

View File

@ -0,0 +1,56 @@
{
"intent": "Bring phone_focus_mode/lib/adb_common.sh (346 lines) under the repo-wide 250-line cap without changing the API any consumer gets from one source line.",
"scope": [
"phone_focus_mode/lib/adb_common.sh — reduced to logging, device selection, root and identity helpers, plus two source lines",
"phone_focus_mode/lib/adb_trusted.sh — new: the enrolled-device record (save, list, forget, match, verify)",
"phone_focus_mode/lib/adb_locking.sh — new: the run lock and per-marker cooldowns",
"Non-goal: the other over-cap phone_focus_mode shell files, whose tests are source-text greps rather than behavioural (see risks)"
],
"changes": [
"Split the two coherent, test-covered groups into siblings that adb_common.sh sources at the end of its own body, so every existing consumer keeps getting the complete API from its single existing `source .../lib/adb_common.sh` line. No consumer was edited.",
"Both new files are sourced via $(cd $(dirname ${BASH_SOURCE[0]}) && pwd), so they resolve relative to the library's own directory rather than the caller's — the failure mode the brief documents for install_leechblock.sh, where an entry whose SCRIPT_DIR has no lib/ dies on its first source. Nothing deploys a copy of this library elsewhere, so that blocker does not apply here.",
"Changed _load_trusted_device_values to take the record path as an optional first argument instead of reading a global that _matches_trusted_record shadowed with a `local`. Once the two functions lived in different files, shellcheck could no longer see that link and reported SC2034. Passing the path explicitly removes the cause rather than suppressing the finding, and is what the caller meant anyway; the default keeps the legacy single-device record working.",
"The two `# shellcheck source=` directives are written relative to the script's own directory, matching the repo's source-path=SCRIPTDIR setting in .shellcheckrc."
],
"verification": [
{
"command": "bash phone_focus_mode/lib/tests/test_adb_common.sh",
"result": "pass",
"evidence": "Results: 16 passed, 0 failed — identical to the pre-split baseline. This suite sources the library, mocks adb, and asserts real behaviour including an injection payload reaching _sanitize_device_string literally, so it is a behavioural gate rather than a source-text grep."
},
{
"command": "pre-commit run --files on all three files",
"result": "pass",
"evidence": "Every hook passes, including shellcheck, which this repo runs with NO --severity filter so even info-level findings fail. Zero suppressions were added: the SC2034 the split initially caused was fixed by changing the function signature, and the SC1091s by correcting the source directives."
},
{
"command": "Sourcing adb_common.sh alone and checking declare -F for functions from all three files",
"result": "pass",
"evidence": "adb_list_serials (kept), adb_save_trusted_device and adb_verify_trusted_identity (moved to adb_trusted.sh), adb_acquire_lock and adb_check_cooldown (moved to adb_locking.sh) are all defined. This is the check that proves the split is transparent to consumers rather than merely syntactically valid."
},
{
"command": "bash -n on run_phone.sh, phone_backup.sh, distraction_purge.sh",
"result": "pass",
"evidence": "All three real consumers still parse."
},
{
"command": "Function coverage measurement, run before choosing the seam",
"result": "pass",
"evidence": "13 of 23 functions (56%) in adb_common.sh are named by the test file. Both extracted groups sit inside the covered set — all four locking functions and five of six trusted-device functions are exercised — so the 16 tests genuinely verify the moved code rather than passing around it."
},
{
"command": "bash ~/utils/scripts/check_file_length.sh --all",
"result": "pass",
"evidence": "adb_common.sh 155, adb_trusted.sh 136, adb_locking.sh 76 — none appears in the over-cap listing, down from a single 346-line file."
}
],
"risks": [
"adb_trusted.sh and adb_locking.sh are not independently sourceable: both read globals and call logging helpers that adb_common.sh defines first. Each carries a header comment saying so. Sourcing one directly would fail under set -u rather than misbehave silently.",
"The _load_trusted_device_values signature change is the one behavioural edit in this commit. Its only in-repo caller now passes the path explicitly; the default preserves the previous global-reading behaviour for any caller that does not.",
"The other over-cap files in this package (focus_ctl.sh 1091, deploy.sh 833, config.sh 571, focus_daemon.sh 543, monitor.sh 449) are NOT split here. test_dns_enforcer.sh, test_monitor.sh and test_tether_enforcer.sh contain zero assertions of the form this suite uses, so those files have no behavioural gate — splitting them would be unverifiable in exactly the way the brief warns about."
],
"rollback": [
"git revert the commit — one edited file and two added files, with no data, config or schema migration.",
"After rollback, re-run bash phone_focus_mode/lib/tests/test_adb_common.sh; expect 16 passed, 0 failed."
]
}

View File

@ -46,8 +46,13 @@ adb_list_serials() {
awk 'NR > 1 && $2 ~ /^(device|offline|unauthorized)$/ { print $1 }'
}
# Reads one enrolled-device record into TRUSTED_*_LOADED.
# Takes the path explicitly, defaulting to the legacy single-device record.
# Callers that need a specific record pass it rather than shadowing the global,
# which kept the value from leaking into later calls without relying on a
# `local` of a name declared in another file.
_load_trusted_device_values() {
local file_path="${TRUSTED_DEVICE_FILE}"
local file_path="${1:-${TRUSTED_DEVICE_FILE}}"
local -a loaded_values=()
[[ -f "${file_path}" ]] || return 1
@ -144,203 +149,12 @@ adb_collect_identity() {
export DEVICE_MODEL DEVICE_FINGERPRINT DEVICE_SERIAL
}
ADB_ENROLLED_DEVICE_FILE=""
# The enrolled-device record. Sourced from this file's own directory so every
# existing consumer keeps getting the whole API from one source line.
# shellcheck source=adb_trusted.sh
source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/adb_trusted.sh"
# Path of the per-device record for a serial. Serials are sanitised to
# [A-Za-z0-9._-] by _sanitize_device_string, so this cannot escape the dir.
_trusted_device_path() {
printf '%s/%s.sh' "${TRUSTED_DEVICE_DIR}" "$1"
}
adb_save_trusted_device() {
local record=""
adb_collect_identity
mkdir -p "${TRUSTED_DEVICE_DIR}"
chmod 700 "${TRUSTED_DEVICE_DIR}"
record="$(_trusted_device_path "${DEVICE_SERIAL}")"
{
printf '# Auto-generated trusted device record — do not edit manually.\n'
printf 'TRUSTED_SERIAL=%q\n' "${DEVICE_SERIAL}"
printf 'TRUSTED_MODEL=%q\n' "${DEVICE_MODEL}"
printf 'TRUSTED_FINGERPRINT=%q\n' "${DEVICE_FINGERPRINT}"
} >"${record}"
chmod 600 "${record}"
ADB_ENROLLED_DEVICE_FILE="${record}"
export ADB_ENROLLED_DEVICE_FILE
_info "Enrolled device: model='${DEVICE_MODEL}' serial='${DEVICE_SERIAL}'"
}
# Serials with a per-device record, newest first is not meaningful here so
# the order is whatever the glob yields.
adb_list_trusted_serials() {
local record=""
[[ -d "${TRUSTED_DEVICE_DIR}" ]] || return 0
for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do
[[ -e "${record}" ]] || continue
basename "${record}" .sh
done
}
adb_forget_trusted_device() {
local serial="${1:?serial required}"
local record=""
record="$(_trusted_device_path "$(_sanitize_device_string "${serial}")")"
if [[ -f "${record}" ]]; then
rm -f "${record}"
_info "Removed trusted device record for '${serial}'"
else
_warn "No trusted device record for '${serial}'"
fi
}
# Compares the connected device against one record. Returns 0 on a match,
# 1 when the record is for a different device, and 2 when the record names
# this serial but its model or fingerprint has changed -- which is the case
# worth shouting about, since a serial is the easiest field to spoof and a
# changed fingerprint means the device was reflashed.
_matches_trusted_record() {
local record="${1:?record required}"
local saved_serial="" saved_model="" saved_fingerprint=""
# Scoped to this call: _load_trusted_device_values reads the global, and
# leaking a per-record value would change which file later callers use.
local TRUSTED_DEVICE_FILE="${record}"
if ! _load_trusted_device_values; then
_fatal "Trusted device record exists but could not be read: ${record}"
fi
saved_serial="${TRUSTED_SERIAL_LOADED:-}"
saved_model="${TRUSTED_MODEL_LOADED:-}"
saved_fingerprint="${TRUSTED_FINGERPRINT_LOADED:-}"
[[ -n "${saved_serial}" && "${DEVICE_SERIAL}" != "${saved_serial}" ]] && return 1
if [[ -n "${saved_model}" && "${DEVICE_MODEL}" != "${saved_model}" ]]; then
_error "Device '${DEVICE_SERIAL}' is enrolled but its model changed: expected '${saved_model}', got '${DEVICE_MODEL}'."
return 2
fi
if [[ -n "${saved_fingerprint}" && "${DEVICE_FINGERPRINT}" != "${saved_fingerprint}" ]]; then
_error "Device '${DEVICE_SERIAL}' is enrolled but its build fingerprint changed."
_error " expected: ${saved_fingerprint}"
_error " got: ${DEVICE_FINGERPRINT}"
return 2
fi
return 0
}
# Fails closed: an unknown device aborts. Any ENROLLED device is accepted,
# so several phones can be managed from one PC without re-enrolling on
# every switch. Only a completely empty store is permissive, and that is
# the first-run case where there is nothing yet to contradict.
adb_verify_trusted_identity() {
local record="" rc=0
local -a records=()
if [[ -d "${TRUSTED_DEVICE_DIR}" ]]; then
for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do
[[ -e "${record}" ]] && records+=("${record}")
done
fi
# Legacy single-device record stays authoritative until it is migrated,
# so an existing install keeps its guarantee without any manual step.
[[ -f "${_PHONE_STATE_DIR}/trusted_device.sh" ]] &&
records+=("${_PHONE_STATE_DIR}/trusted_device.sh")
if [[ ${#records[@]} -eq 0 ]]; then
_warn "No trusted device record found. Run 'fresh-phone' to enroll this device."
return 0
fi
adb_collect_identity
for record in "${records[@]}"; do
_matches_trusted_record "${record}" && {
_info "Device identity verified: ${DEVICE_SERIAL} (${DEVICE_MODEL})"
return 0
}
rc=$?
# A tampered/reflashed match must abort rather than fall through to
# the next record, which would silently downgrade the check.
[[ "${rc}" -eq 2 ]] && _fatal "Refusing to proceed automatically."
done
_error "Device '${DEVICE_SERIAL}' (${DEVICE_MODEL}) is not enrolled."
_error "Enrolled: $(adb_list_trusted_serials | tr '\n' ' ')"
_fatal "Refusing to proceed automatically. Enroll it first if this is expected."
}
_adb_release_lock() {
if [[ -n "${LOCK_FILE}" && -d "${LOCK_FILE}" ]]; then
rm -rf "${LOCK_FILE}"
fi
}
adb_acquire_lock() {
local lock_pid_file=""
local old_pid=""
mkdir -p "${LOCK_DIR}"
LOCK_FILE="${LOCK_DIR}/run_phone.lock"
lock_pid_file="${LOCK_FILE}/pid"
if mkdir "${LOCK_FILE}" 2>/dev/null; then
printf '%s\n' "$$" >"${lock_pid_file}"
trap _adb_release_lock EXIT INT TERM
_info "Acquired run lock (PID $$)"
return 0
fi
old_pid="$(cat "${lock_pid_file}" 2>/dev/null || printf '')"
if [[ -n "${old_pid}" ]] && kill -0 "${old_pid}" 2>/dev/null; then
_fatal "Another run_phone.sh instance is already running (PID ${old_pid}). Aborting."
fi
_warn "Stale lock directory found (PID ${old_pid} no longer running). Removing."
rm -rf "${LOCK_FILE}"
if ! mkdir "${LOCK_FILE}" 2>/dev/null; then
_fatal "Could not acquire run lock at ${LOCK_FILE}. Another process may have raced us."
fi
printf '%s\n' "$$" >"${lock_pid_file}"
trap _adb_release_lock EXIT INT TERM
_info "Acquired run lock (PID $$)"
}
adb_check_cooldown() {
local cooldown_secs="${1:-300}"
local elapsed=0
local last_run=0
local marker_name="${2:-default}"
local marker="${LAST_RUN_DIR}/${marker_name}"
local now=0
mkdir -p "${LAST_RUN_DIR}"
if [[ -f "${marker}" ]]; then
last_run="$(cat "${marker}")"
if [[ "${last_run}" =~ ^[0-9]+$ ]]; then
now="$(date +%s)"
elapsed=$((now - last_run))
if ((elapsed < cooldown_secs)); then
_info "Cooldown active: last run ${elapsed}s ago, cooldown is ${cooldown_secs}s. Skipping."
return 1
fi
else
_warn "Ignoring invalid cooldown marker: ${marker}"
fi
fi
return 0
}
adb_mark_last_run() {
local marker_name="${1:-default}"
mkdir -p "${LAST_RUN_DIR}"
date +%s >"${LAST_RUN_DIR}/${marker_name}"
}
# Run lock and cooldown helpers. Sourced last, from this file's own directory,
# so every existing consumer keeps getting the whole API from one source line.
# shellcheck source=adb_locking.sh
source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/adb_locking.sh"

View File

@ -0,0 +1,76 @@
#!/usr/bin/env bash
# lib/adb_locking.sh — single-instance run lock and per-marker cooldowns.
# Sourced by adb_common.sh; do not execute directly, and do not source it
# on its own: it reads LOCK_DIR, LAST_RUN_DIR and LOCK_FILE, and calls the
# _info/_warn/_fatal helpers, all of which adb_common.sh defines first.
_adb_release_lock() {
if [[ -n "${LOCK_FILE}" && -d "${LOCK_FILE}" ]]; then
rm -rf "${LOCK_FILE}"
fi
}
adb_acquire_lock() {
local lock_pid_file=""
local old_pid=""
mkdir -p "${LOCK_DIR}"
LOCK_FILE="${LOCK_DIR}/run_phone.lock"
lock_pid_file="${LOCK_FILE}/pid"
if mkdir "${LOCK_FILE}" 2>/dev/null; then
printf '%s\n' "$$" >"${lock_pid_file}"
trap _adb_release_lock EXIT INT TERM
_info "Acquired run lock (PID $$)"
return 0
fi
old_pid="$(cat "${lock_pid_file}" 2>/dev/null || printf '')"
if [[ -n "${old_pid}" ]] && kill -0 "${old_pid}" 2>/dev/null; then
_fatal "Another run_phone.sh instance is already running (PID ${old_pid}). Aborting."
fi
_warn "Stale lock directory found (PID ${old_pid} no longer running). Removing."
rm -rf "${LOCK_FILE}"
if ! mkdir "${LOCK_FILE}" 2>/dev/null; then
_fatal "Could not acquire run lock at ${LOCK_FILE}. Another process may have raced us."
fi
printf '%s\n' "$$" >"${lock_pid_file}"
trap _adb_release_lock EXIT INT TERM
_info "Acquired run lock (PID $$)"
}
adb_check_cooldown() {
local cooldown_secs="${1:-300}"
local elapsed=0
local last_run=0
local marker_name="${2:-default}"
local marker="${LAST_RUN_DIR}/${marker_name}"
local now=0
mkdir -p "${LAST_RUN_DIR}"
if [[ -f "${marker}" ]]; then
last_run="$(cat "${marker}")"
if [[ "${last_run}" =~ ^[0-9]+$ ]]; then
now="$(date +%s)"
elapsed=$((now - last_run))
if ((elapsed < cooldown_secs)); then
_info "Cooldown active: last run ${elapsed}s ago, cooldown is ${cooldown_secs}s. Skipping."
return 1
fi
else
_warn "Ignoring invalid cooldown marker: ${marker}"
fi
fi
return 0
}
adb_mark_last_run() {
local marker_name="${1:-default}"
mkdir -p "${LAST_RUN_DIR}"
date +%s >"${LAST_RUN_DIR}/${marker_name}"
}

View File

@ -0,0 +1,135 @@
#!/usr/bin/env bash
# lib/adb_trusted.sh — the enrolled-device record: saving, listing, forgetting,
# and refusing to act on a phone that is not enrolled.
# Sourced by adb_common.sh; do not execute directly, and do not source it on
# its own: it uses TRUSTED_DEVICE_DIR/TRUSTED_DEVICE_FILE, the DEVICE_* vars and
# the _info/_warn/_error/_fatal helpers that adb_common.sh defines first.
ADB_ENROLLED_DEVICE_FILE=""
# Path of the per-device record for a serial. Serials are sanitised to
# [A-Za-z0-9._-] by _sanitize_device_string, so this cannot escape the dir.
_trusted_device_path() {
printf '%s/%s.sh' "${TRUSTED_DEVICE_DIR}" "$1"
}
adb_save_trusted_device() {
local record=""
adb_collect_identity
mkdir -p "${TRUSTED_DEVICE_DIR}"
chmod 700 "${TRUSTED_DEVICE_DIR}"
record="$(_trusted_device_path "${DEVICE_SERIAL}")"
{
printf '# Auto-generated trusted device record — do not edit manually.\n'
printf 'TRUSTED_SERIAL=%q\n' "${DEVICE_SERIAL}"
printf 'TRUSTED_MODEL=%q\n' "${DEVICE_MODEL}"
printf 'TRUSTED_FINGERPRINT=%q\n' "${DEVICE_FINGERPRINT}"
} >"${record}"
chmod 600 "${record}"
ADB_ENROLLED_DEVICE_FILE="${record}"
export ADB_ENROLLED_DEVICE_FILE
_info "Enrolled device: model='${DEVICE_MODEL}' serial='${DEVICE_SERIAL}'"
}
# Serials with a per-device record, newest first is not meaningful here so
# the order is whatever the glob yields.
adb_list_trusted_serials() {
local record=""
[[ -d "${TRUSTED_DEVICE_DIR}" ]] || return 0
for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do
[[ -e "${record}" ]] || continue
basename "${record}" .sh
done
}
adb_forget_trusted_device() {
local serial="${1:?serial required}"
local record=""
record="$(_trusted_device_path "$(_sanitize_device_string "${serial}")")"
if [[ -f "${record}" ]]; then
rm -f "${record}"
_info "Removed trusted device record for '${serial}'"
else
_warn "No trusted device record for '${serial}'"
fi
}
# Compares the connected device against one record. Returns 0 on a match,
# 1 when the record is for a different device, and 2 when the record names
# this serial but its model or fingerprint has changed -- which is the case
# worth shouting about, since a serial is the easiest field to spoof and a
# changed fingerprint means the device was reflashed.
_matches_trusted_record() {
local record="${1:?record required}"
local saved_serial="" saved_model="" saved_fingerprint=""
# Passed explicitly rather than shadowing the global, so a per-record value
# cannot change which file later callers read.
if ! _load_trusted_device_values "${record}"; then
_fatal "Trusted device record exists but could not be read: ${record}"
fi
saved_serial="${TRUSTED_SERIAL_LOADED:-}"
saved_model="${TRUSTED_MODEL_LOADED:-}"
saved_fingerprint="${TRUSTED_FINGERPRINT_LOADED:-}"
[[ -n "${saved_serial}" && "${DEVICE_SERIAL}" != "${saved_serial}" ]] && return 1
if [[ -n "${saved_model}" && "${DEVICE_MODEL}" != "${saved_model}" ]]; then
_error "Device '${DEVICE_SERIAL}' is enrolled but its model changed: expected '${saved_model}', got '${DEVICE_MODEL}'."
return 2
fi
if [[ -n "${saved_fingerprint}" && "${DEVICE_FINGERPRINT}" != "${saved_fingerprint}" ]]; then
_error "Device '${DEVICE_SERIAL}' is enrolled but its build fingerprint changed."
_error " expected: ${saved_fingerprint}"
_error " got: ${DEVICE_FINGERPRINT}"
return 2
fi
return 0
}
# Fails closed: an unknown device aborts. Any ENROLLED device is accepted,
# so several phones can be managed from one PC without re-enrolling on
# every switch. Only a completely empty store is permissive, and that is
# the first-run case where there is nothing yet to contradict.
adb_verify_trusted_identity() {
local record="" rc=0
local -a records=()
if [[ -d "${TRUSTED_DEVICE_DIR}" ]]; then
for record in "${TRUSTED_DEVICE_DIR}"/*.sh; do
[[ -e "${record}" ]] && records+=("${record}")
done
fi
# Legacy single-device record stays authoritative until it is migrated,
# so an existing install keeps its guarantee without any manual step.
[[ -f "${_PHONE_STATE_DIR}/trusted_device.sh" ]] &&
records+=("${_PHONE_STATE_DIR}/trusted_device.sh")
if [[ ${#records[@]} -eq 0 ]]; then
_warn "No trusted device record found. Run 'fresh-phone' to enroll this device."
return 0
fi
adb_collect_identity
for record in "${records[@]}"; do
_matches_trusted_record "${record}" && {
_info "Device identity verified: ${DEVICE_SERIAL} (${DEVICE_MODEL})"
return 0
}
rc=$?
# A tampered/reflashed match must abort rather than fall through to
# the next record, which would silently downgrade the check.
[[ "${rc}" -eq 2 ]] && _fatal "Refusing to proceed automatically."
done
_error "Device '${DEVICE_SERIAL}' (${DEVICE_MODEL}) is not enrolled."
_error "Enrolled: $(adb_list_trusted_serials | tr '\n' ' ')"
_fatal "Refusing to proceed automatically. Enroll it first if this is expected."
}