testsAndMisc/docs/DOCS-shell-split-verification.md
Krzysztof kuhy Rudnicki 49d82511c3 Adopt the four markdown namespaces and gate them
Renames this repo's markdown onto README.md / CLAUDE*.md / DOCS*.md /
TODO*.md so a future session can tell a live task from a finished
record without reading the file to find out.

TODO files carry the literal "REMOVE ME AFTER FINISH" and are deleted
when the work lands. Enforced by the md-naming pre-commit hook and CI
workflow, both delegating to the shared gate in kuhyx/utils so this
repo's idea of a valid name cannot drift from every other repo's.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A47aX5DJwmA3QFn3pkEWi7
2026-08-24 15:55:46 +02:00

174 lines
9.0 KiB
Markdown

# Shell split verification: how a green split is still broken
Split out of `docs/shell-split-recipes.md` when that file hit the same 250-line
cap it exists to serve. That file covers **how to make** a split; this one
covers **how to find out it is wrong**. Read both before the next
`linux_configuration` or `phone_focus_mode` split.
Every section below cost a failed run, a revert, or — once — a live system
mutation.
### A `--skip-install` flag is not evidence the script is safe to run
`shell_check.sh` gates only `install_if_missing`'s pacman branch on
`SKIP_INSTALL`. The AUR branch inside `install_linters` runs regardless, so a
backgrounded "baseline" run installed four packages before it was killed
mid-`makepkg`. Decision 6 was violated by a flag that looked like permission.
Before running any script to capture a baseline, grep the flag's variable and
confirm it gates **every** mutation path, not just the first one:
```bash
grep -n 'SKIP_INSTALL\|DRY_RUN' <script> # then read each hit
```
`--list-only` was the genuinely safe path here, because it returns before
`install_linters` is reached at all.
### Check whether the target is DEPLOYED as a single file — the trace cannot
Before designing seams, find out whether some other script copies the target
somewhere and runs it from there. An entry+`lib/` shape ships an entry whose
deployed `SCRIPT_DIR` has no `lib/`, so it dies on its first `source` under
`set -e` — while your trace stays green, because the trace runs the repo copy
where `lib/` exists.
```bash
grep -rn '<target-basename>' linux_configuration/ --include='*.sh' \
| grep -v '<the target itself>:'
```
Confirmed blocked on this, not theoretically: `install_leechblock.sh` (the
pacman installer `cp`s it to `/usr/local/share/digital_wellbeing/` and
`pacman_wrapper.sh:831` prefers that copy on **every pacman invocation**) and
`block_compulsive_opening.sh` (`install_all` copies itself to
`/usr/local/bin`). A target invoked as `$CONFIG_DIR/scripts/...` is fine.
### A stubbed run cannot reach a bug in a seam that passes state
`meta/scripts/verify_shell_split.sh` and the hand-rolled `declare -F` probe
both **source the libs and never call the functions**. That is enough to prove
every `source` line resolves, and nothing more. The `analyze_repo.sh` split
passed `bash -n`, `shellcheck`, a function-set diff, a line-set diff and the
stubbed probe — and still shipped a script that aborted immediately after
language detection. Two separate bugs, neither reachable without executing:
- **`set -e` and the function tail.** Eight `((LANG_FILES[x] > 0)) && HAS_Y=true`
lines were top-level statements, where a false `&&` merely leaves `$?`
non-zero. Wrapped into a function they become its _return value_, and the
last one (java, false for most repos) killed the script. Any time you wrap
former top-level code in a function, check what its **last statement** is;
append `|| true` if it is a bare conditional.
- **A self-referencing nameref.** `local -n LANG_CODE_FILES="$1"` pointing at a
global of the same name still moves data on bash 5.3, but warns
`circular name reference` on _every access_ — straight into the script's
output. Name the local something else (`_code_files`).
So: when a seam passes state, run the thing. Stub only the parts that mutate
the system, and diff the generated artifacts against a detached worktree at the
pre-split commit. That comparison is what proved the fix — same file set, every
text artifact byte-identical.
## The harness and the fixtures
Everything about _running_ the check — `trace_shell_split.sh`, the `--prefix`
mode for scripts whose job is placing files, the regression fixtures, the
entry-point and `shfmt` traps — moved to
**`docs/shell-split-harness.md`** when this file hit the same 250-line cap it
exists to serve. This file stays the _why_; that one is the _how_.
### Prove the move was verbatim: `meta/scripts/verify_shell_split.sh`
The Python splits in this repo are verified with a throwaway `ast` identity
check. There is no `ast` for bash, so the shell equivalent normalises through
`shfmt -mn` (minify: strips comments and indentation, keeps logic) and compares
a hash per top-level function:
```bash
bash meta/scripts/verify_shell_split.sh HEAD <old-path> <new-path>...
# IDENTICAL: all N top-level functions moved verbatim
```
It reports which function changed, so a diff points at the seam rather than the
whole file. Run it after every split, and again after `shfmt`/pre-commit
autofixes — reordering an import or reflowing a line is exactly what it is
designed to see through.
**Verify the harness before trusting it.** A normalizer that silently passes
everything is worse than none. The three checks it must survive:
| input | expected |
| ------------------------------------------------ | --------------------------------------- |
| unchanged copy of the file | `IDENTICAL` |
| comments/blank lines/indentation changed | `IDENTICAL` |
| one character of logic changed (`"$1"` → `"$2"`) | `DIFFERENCE FOUND`, naming the function |
The first attempt at that mutation test used `sed 's/exit 1/exit 2/'` on a file
containing no `exit 1`. It reported `IDENTICAL` — not because the harness was
broken, but because the mutation never applied. Confirm the mutation actually
changed the file (`diff` it) before concluding anything about the harness.
### Deployment triage of the over-cap scripts (2026-08-17)
Run before designing any seam, per the section above. `deploy.sh` copies a
**hardcoded per-file list** into `/data/local/tmp/focus_mode` and never deploys
`lib/`:
- **`phone_focus_mode/*` (8 over-cap scripts)** — deployed individually.
A new sibling must be added to `deploy.sh`'s copy list **in the same commit**,
or focus mode dies on the phone at its first `source`. These scripts source
only `config.sh`, which is on that list, so a sibling-file seam is workable —
an entry+`lib/` seam is not.
- **`install_leechblock.sh`, `block_compulsive_opening.sh`** — copied to
`/usr/local/…` and preferred there by `pacman_wrapper.sh:831` on every pacman
invocation. Teach the installer to deploy the directory first, in its own
commit. Highest blast radius; do these last.
- **`hosts/install.sh`, `setup_hosts_guard.sh`, `lib/monitor.sh`** — install
helper scripts to `/usr/local/sbin` or `/usr/local/bin`; check whether the
file being split is itself the deployed one.
- **The other 16** (`fresh-install/main.sh`, `nvidia_troubleshoot.sh`,
`setup_passwordless_system.sh`, `clean_audio.sh`, `enforce_vbox_hosts.sh`,
`migrate_hosts_guard_to_guard_lib.sh`, `libre_translate.sh`,
`diagnose_pacman_hook_stall.sh`, `install_plagiarism_tools.sh`,
`steam_compatibility.sh`, `deploy.sh`, `setup_night_lockdown.sh`,
`pacman_wrapper.sh`, `generate_study_materials.sh`,
`check_and_enable_services.sh`, `setup_midnight_shutdown.sh`) — no deployed
copy found, so an entry+`lib/` shape is safe.
### A lib must contain the readers of every global it assigns
`install_pacman_wrapper.sh:29` records the constraint: a **definitions-only**
lib "assigns without referencing, which is SC2034, and the repo forbids
suppressions." The rule that follows from it is not "assignments stay in the
entry script" — it is that a file must not assign a variable it never reads.
A lib holding both the writer and its readers is clean.
Getting this inverted is easy and costs several revert cycles.
`libre_translate.sh` was split with `parse_args` (the writer) left in the entry
and `health_check` / `sample_request` / `start_container_ephemeral` (the
readers) moved to the lib — the worst of both, and SC2034 fired on nine
globals. The fix is to colocate: move the writer **and** its consumers into the
same file.
The pre-commit hook is `shellcheck` with **no `-x`**, so each file is analysed
standalone and a `# shellcheck source=` directive will not make the checker
follow the link. Verify by running `shellcheck <lib>` on its own before
committing; the hook will not be any more forgiving.
This is the shell analogue of the `monkeypatch.setattr` target trap: the
question is always which file's namespace owns the name.
**`libre_translate.sh` (488) is parked.** What remains after its clean seams
are taken is `parse_args` (111 lines), `write_env_file`, `detect_container_user`
and `main`, which between them assign and read the same configuration globals.
Getting the entry under the cap needs `parse_args` restructured rather than
relocated — a refactor, not a verbatim move, so it is out of scope for this
method. Attempt it with a test harness, or accept an entry that stays over.
### kcov silently under-reports: see `docs/kcov-under-report.md`
Split out on 2026-08-22 when this file hit the 250-line cap. **Read it before
trusting any coverage percentage**: two distinct kcov defects are documented
there, one confirmed against an independent instrument, along with seven
hypotheses that are already DISPROVEN and must not be retried.