Skip to content

Fix all ShellCheck findings and restore green lint/build CI - #114

Open
RchGrav wants to merge 12 commits into
mainfrom
fix/shellcheck-clean
Open

RchGrav wants to merge 12 commits into
mainfrom
fix/shellcheck-clean

Conversation

@RchGrav

@RchGrav RchGrav commented Sep 15, 2026 •

Copy link
Copy Markdown
Owner

Result

ShellCheck 0.9.0: 425 baseline diagnostics → 0, checking every shell script. The original core-only invocation reported 269 diagnostics; the full 33-script baseline also included builders, container scripts, tests, and tooling. Four new regression scripts bring current coverage to 37 shell scripts.

Changes

  • Separate declarations from command substitutions so failures retain their exit status; quote scalar arguments and Docker bind paths.
  • Correct printf formatting, array handling in profile display, and directory enumeration/counting.
  • Add source hints and narrowly documented annotations for shared module constants, child-shell code, EXIT callbacks, and installer archive markers. No global diagnostic exclusions or severity downgrade.
  • Use argument arrays for the two installer checksum backends.
  • Write the Python-tools completion marker only after successful installation.
  • Repair Dockerfile assertions that previously failed to fail the test, and exercise the selected Bash interpreter in CLI tests.
  • Update CI to lint all shell scripts, remove the nonexistent lib/commands/*.sh target, test dist/claudebox.run, and replace the obsolete artifact upload action.
  • Add four regression scripts with eight checks. Packaged-install CI runs all nine test suites on Ubuntu and macOS.

Verification

  • Fresh local full ShellCheck invocation: exit 0, zero diagnostics.
  • All 68 checks pass under real Bash 3.2.57 and 5.2, including failures propagated from lookups, Python installation success/failure, multi-profile output, and both checksum backends.
  • The same 68 checks passed against fresh packaged installations, with repeat installer invocation verified.
  • Dockerfile mutation check: unresolved placeholders passed the old assertion but fail the corrected test.
  • git diff --check passes.
  • The uploaded Git tree hash exactly matches the locally tested tree: ad24a99b3750079e58276e5d275f53166d619174.

Runtime tests execute the real launcher and jq with Docker stubbed at the boundary; this does not claim full Claude application-image integration testing. GitHub CI results will be verified before merging.

Temporary tool-bootstrap workflow commits leave no file in the final diff; squash on merge.

Summary by Sourcery

Eliminate ShellCheck diagnostics across the shell codebase and harden the installer, runtime behavior, regression coverage, and CI validation.

Bug Fixes:

  • Preserve command failure status, correct shell output and formatting, improve path and checksum argument handling, and ensure installation markers are written only after successful setup.
  • Strengthen clipboard bridge validation and diagnostics, restore caller signal traps, and enforce valid clipboard server limits.
  • Correct Dockerfile assertions and CLI test interpreter coverage to prevent false-positive runtime checks.

Enhancements:

  • Make shell behavior robust and portable across Bash 3.2 and newer versions while removing unsafe or unreliable command patterns.
  • Expand regression coverage for shell failures, profile output, Python installation status, installer checksum backends, and runtime behavior.

Build:

  • Update installer packaging and generated launcher handling, including safer archive metadata and checksum backend invocation.

CI:

  • Replace baseline ShellCheck checks with full linting of all shell scripts and expand packaged-install test execution across supported platforms.
  • Refresh artifact upload configuration and validate the packaged installer, repeat installation, and complete regression suites in CI.

Documentation:

  • Document full ShellCheck coverage and the expanded regression test suites.

Tests:

  • Add regression scripts covering shell status propagation, Python installation completion, profile rendering, and installer checksum behavior.
  • Extend Bash 3.2 compatibility, Docker runtime, clipboard, and macOS integration coverage.

Resolve 425 baseline diagnostics without lowering ShellCheck severity or adding global exclusions. Preserve failed command-substitution statuses, quote arguments, fix formatted output and installer checksum argument handling, and identify cross-module data and trap callbacks with narrow documented annotations.

Fix Python-tool completion markers after failed installs and ineffective Dockerfile test assertions. Add eight regression checks covering failure propagation, Python install results, output rendering, and both checksum backends. Check all shell scripts in CI and use the actual dist installer and supported artifact action.

Verified matching local/remote tree ad24a99: ShellCheck 0.9.0 reports zero diagnostics; 68 checks pass under Bash 3.2.57 and 5.2. Runtime Docker calls are stubbed at the boundary, not full application-image integration.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @RchGrav, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 7 days by commenting @sourcery-ai review. Upgrade to get a review now.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-15T22:30:18.107073Z cb19712 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@sourcery-ai

sourcery-ai Bot commented Sep 15, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR drives ShellCheck diagnostics to zero across all shell scripts by refactoring status-sensitive declarations, quoting and formatting unsafe expansions, replacing fragile enumeration logic, and documenting only intentional exceptions. It also strengthens installer/build behavior and adds comprehensive Bash 3.2/5.2, packaged-install, runtime, output, Dockerfile, Python-install, and checksum regression coverage, with CI updated to exercise the complete surface.

Sequence diagram for packaged installer checksum validation

sequenceDiagram
    participant User
    participant Installer as dist_claudebox_run
    participant Archive as InstallArchive
    participant Setup as setup_sh
    participant Checksum as sha256sum_or_shasum

    User->>Installer: invoke installer
    Installer->>Checksum: calculate archive checksum
    Checksum-->>Installer: checksum value
    alt checksum matches
        Installer->>Installer: extract archive
        Installer->>Setup: exec setup.sh
        Setup-->>User: installation starts
    else checksum missing or mismatched
        Installer->>Installer: refresh archive and checksum
        Installer->>Setup: exec setup.sh
    end
Loading

Flow diagram for failure-safe shell installation status

flowchart TD
    A[Start Python tools installation] --> B[Install Python tools]
    B -->|success| C[Write completion marker]
    B -->|failure| D[Return failure]
    C --> E[Future runs detect completed installation]
    D --> F[Marker remains absent]
    F --> G[Future runs retry installation]
Loading

File-Level Changes

Change Details Files
Preserve shell failure status and eliminate unsafe or ambiguous ShellCheck patterns across runtime, build, installer, and tooling scripts.
  • Split declarations from command substitutions so assignment failures propagate under set -e.
  • Quote scalar expansions, numeric tests, Docker bind mounts, dynamic printf formats, and command arguments.
  • Replace fragile ls-based enumeration/counting and arithmetic side effects with explicit loops and assignments.
  • Add source directives and narrowly scoped annotations for intentional cross-file, child-shell, trap, and archive-marker behavior.
main.sh
lib/cli.sh
lib/commands.clean.sh
lib/commands.core.sh
lib/commands.info.sh
lib/commands.profile.sh
lib/commands.sh
lib/commands.slot.sh
lib/commands.system.sh
lib/common.sh
lib/config.sh
lib/docker.sh
lib/env.sh
lib/os.sh
lib/preflight.sh
lib/project-fixed.sh
lib/project.sh
lib/state.sh
tooling/profiles/rust.sh
.builder/build.sh
.builder/script_template.sh
.builder/script_template_root.sh
build/docker-entrypoint
build/generate-tools-readme
build/init-firewall
Harden installer behavior and packaging validation, including portable checksum invocation and success-only installation markers.
  • Use argument arrays for sha256sum and shasum -a 256 backends.
  • Keep archive markers data-only while documenting intentional ShellCheck suppressions.
  • Verify packaged dist/claudebox.run, checksum backends, and repeat installer invocation in CI.
  • Write the Python-tools completion marker only after successful installation.
.builder/script_template.sh
.builder/script_template_root.sh
build/docker-entrypoint
.github/workflows/build.yml
.github/workflows/test.yml
tests/test_installer_checksum.sh
tests/test_python_install_status.sh
Correct output rendering, profile parsing, directory accounting, and generated Dockerfile assertions.
  • Parse profile names line-by-line to preserve array elements and independently render enabled profiles.
  • Use safe printf formats for colorized output and count matching files/directories explicitly.
  • Make unresolved Dockerfile placeholders cause an explicit test failure.
lib/commands.info.sh
lib/commands.profile.sh
lib/common.sh
main.sh
tests/test_dockerfile_template.sh
tests/test_shell_output.sh
Expand regression and compatibility coverage while updating CI to lint and test the complete shell-script surface.
  • Add robustness, Python status, shell output, and installer checksum regression suites.
  • Run the expanded suites under Bash 3.2 and the selected Bash interpreter, including packaged-install tests on Ubuntu and macOS.
  • Lint all 37 shell scripts, remove the obsolete command-module target, update checkout/artifact actions, and document the verification commands.
.github/workflows/build.yml
.github/workflows/test.yml
tests/README.md
tests/test_bash32_compat.sh
tests/test_cli_bash32.sh
tests/test_in_bash32_docker.sh
tests/test_runtime_regressions.sh
tests/test_shell_robustness.sh
tests/test_python_install_status.sh
tests/test_shell_output.sh
tests/test_installer_checksum.sh

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@RchGrav

RchGrav commented Sep 17, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

Copilot AI and others added 8 commits September 17, 2026 03:20
# Conflicts:
#	.github/workflows/build.yml
#	.github/workflows/test.yml
#	build/docker-entrypoint
#	build/init-firewall
#	lib/docker.sh
#	lib/preflight.sh
#	tests/README.md
#	tests/test_dockerfile_template.sh

Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>
Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>
Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>
Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>
Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>
Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>
Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>
Co-authored-by: RchGrav <643233+RchGrav@users.noreply.github.com>

Copilot AI commented Sep 17, 2026

Copy link
Copy Markdown

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts and pushed the merged branch updates in 77954ac.

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