Skip to content

elegance: install.sh has no source-file precheck — partial install + lying --dry-run on a missing/renamed theme#19

Merged
Antawari merged 1 commit into
mainfrom
catrina/2026-06-13/w1-candyfactory-cosmic-install-source-precheck
Jun 14, 2026
Merged

elegance: install.sh has no source-file precheck — partial install + lying --dry-run on a missing/renamed theme#19
Antawari merged 1 commit into
mainfrom
catrina/2026-06-13/w1-candyfactory-cosmic-install-source-precheck

Conversation

@Antawari

Copy link
Copy Markdown
Contributor

DO NOT MERGE — fleet quality burn unit. Review/merge is the user's call.

The claim this satisfies

install.sh ran cp -f "$HERE/themes/<file>" <target> for five sources with no existence guard.

Reproducer (partial install): delete themes/candyfactory-parlor-light.ron, run ./install.sh. Under set -e the dark theme copies first, then cp errors cannot stat … No such file or directory and the script aborts exit 1 — leaving a half-installed theme set in ~/.config/cosmic with no cleanup and no typed message.

Reproducer (--dry-run lies): ./install.sh --dry-run for the same missing source printed all five would: cp lines and exited 0, falsely reporting success — because run() only echoes in dry mode and never touches the filesystem, so the missing source is never noticed.

Both reproducers confirmed RED on the unmodified installer before the fix (1 partial file written; dry-run exit 0).

The fix

A precheck that asserts all five sources exist (and fails fast with one clear typed message before any copy) closes both paths. It runs in both normal and dry-run modes, so --dry-run can no longer lie. Sources are named once and shared with the copy block, so the precheck and the copies can never drift (DRY). The --uninstall, --help, and happy-path install behaviours are unchanged.

TDD

tests/test_install_precheck.sh (new, dependency-free bash) was written RED-first and pins:

  • real install over a missing source aborts non-zero and writes nothing (no partial install)
  • --dry-run over a missing source exits non-zero and prints no would: cp preview (dry-run no longer lies)
  • the abort message names the missing file
  • happy-path regression guards: full real install still lands the set; full --dry-run still previews and exits 0

Wired into .github/workflows/checks.yml. Complements the existing tests/test_install.sh (happy-path integration) — no overlap, no existing test weakened.

Gate (local, mirrors CI)

  • shellcheck install.sh tests/*.sh — clean
  • test_accent_palettes.sh 14/14 · test_theme_rons.sh 13/13 · test_install_precheck.sh 5/5 · test_install.sh 7/7 — 39/39 green

🤖 Generated with Claude Code

… honest --dry-run

install.sh copied five sources with `run cp -f "$HERE/themes/<file>" <target>`
and no existence guard. With one source missing (a renamed or not-yet-built
theme), under `set -e` the dark theme copied first, then cp aborted the script
exit 1 — leaving a half-installed theme set in ~/.config/cosmic with no cleanup
and no clear message. Worse, `./install.sh --dry-run` printed all five
"would: cp" lines and exited 0 for the same missing source, falsely reporting
success because run() only echoes in dry mode.

Add a precheck that asserts all five sources exist before any copy and fails
fast with one typed message naming each missing file — running in BOTH normal
and dry-run modes. Sources are named once and shared with the copy block so the
precheck and the copies can never drift (DRY). New TDD suite
tests/test_install_precheck.sh pins both closed paths (RED before, GREEN after)
plus happy-path regression guards in both modes; wired into the checks workflow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Antawari Antawari changed the title DO-NOT-MERGE · elegance: install.sh has no source-file precheck — partial install + lying --dry-run on a missing/renamed theme elegance: install.sh has no source-file precheck — partial install + lying --dry-run on a missing/renamed theme Jun 14, 2026
@Antawari Antawari merged commit 42f4f0c into main Jun 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants