Skip to content

refactor(main): split src/main.sh into a src/main/ module #948

Closed

Description

@Chemaclass

Summary

src/main.sh is the last large flat file: 1427 lines, 24 functions. It was carried out of
#931 deliberately — it is the dispatcher and the riskiest file in src/, so it deserves its
own issue rather than being that issue's tail end.

Measured on main (8f2a1c3), after #931 and #940 left src/ at 11 modules + 15 flat files.

What is in it

Function Lines Concern
cmd_test 485 flag parsing for the default subcommand
exec_tests 189 the run lifecycle: dispatch, aggregate, report, exit code
cmd_bench 108 bench flag parsing
cmd_doc 69 doc flag parsing
exec_multi_assert · exec_assert · handle_assert_exit_code · cmd_assert · is_assertion_function · is_exit_code_assertion 213 the standalone bashunit assert path
watch_loop · watch_get_checksum 73 the --watch polling loop
validate_config_or_exit · set_shard_or_exit · require_non_negative_int_or_exit · require_writable_path_or_exit · abort_unknown_option 135 input validation shared by the parsers
cmd_init · cmd_learn · cmd_watch · cmd_upgrade 87 thin delegators to src/cli/
exec_benchmarks · cleanup · handle_stop_on_failure_sync 62 run helpers

Proposed shape

Not pre-decided. Produce the responsibility map first, as every module under #931 did — but
the seams that look real from the function list:

  • validation — the five *_or_exit / abort_* helpers, called by every parser
  • the assert subcommand — six functions, 213 lines, entirely self-contained
  • watch loop — two functions, already delegating to src/cli/watch.sh
  • flag parsing — cmd_*, which is most of the bulk
  • run lifecycle — exec_tests, exec_benchmarks, cleanup, handle_stop_on_failure_sync

Open question worth answering in the map: cmd_doc, cmd_init, cmd_learn, cmd_watch and
cmd_upgrade are flag parsing for subcommands whose implementations already live in
src/cli/. Should each parser move next to its implementation, or stay together as one
parsing layer? Both are defensible; decide deliberately and record it.

Why this one is riskier than the previous seven

  • It is the only caller of src/cli/*, src/learn/, src/benchmark.sh and the runner, so a
    mistake here breaks every subcommand at once rather than one module.
  • refactor(src): group cohesive flat files into modules (assert, console, util) #940 showed subcommand moves interact with build::embed_docs markers and entrypoint
    ordering; cmd_doc sits directly on that path.
  • bashunit sources src/main.sh last, after everything it dispatches to. Any split must
    preserve that position.
  • cmd_test at 485 lines is a single while/case over ~60 flags. Cutting it means cutting
    a case statement, which is the one shape where a mechanical line-range split is unsafe.

Constraints

All of ADR-010's, which by now have all drawn blood at least once:

Verification

Prove it is a relocation: the non-blank line multiset differs only by new shebangs, module
headers and source lines; function count unchanged; the built artifact's sorted code content
identical.

Then: ./bashunit tests/ · --parallel · --parallel --simple --strict · make sa ·
make lint · CI-mode ShellCheck · bash build.sh bin -v printing ✅ Build verified ✅.

Because this file owns dispatch, also smoke-test every subcommand end to end against the
built binary, not just dev mode: test, assert, doc, init, learn, watch,
upgrade, bench.

Acceptance criteria

  • Responsibility map posted here and agreed before any code moves
  • The cmd_*-next-to-implementation question answered explicitly
  • git diff shows only relocations — no renamed functions, no changed logic
  • src/main/index.sh holds only source lines and comments
  • bashunit still sources the module last
  • Every subcommand smoke-tested against the built binary
  • All suites above green, including bash build.sh bin -v
  • .claude/rules/architecture-map.md updated
  • No CHANGELOG entry — internal, no user-visible behaviour change

Do not

  • Do not split cmd_test's case statement across files
  • Do not rename functions
  • Do not move main.sh earlier in the entrypoint's source order
  • Do not run shfmt -w; make lint is the format gate

Activity

  1. Chemaclass commented on Aug 1, 2026

    @Chemaclass
    MemberAuthor

    Sequencing note. #949 groups the remaining 15 flat files into system/, util/, api/, config/ and benchmark/, leaving main.sh as the only loose file in src/. That lands first.

    Two things for this issue to decide once it does:

    1. Whether main.sh becomes src/main/ (with a real main/index.sh aggregator) or stays a thin root dispatcher after its parts distribute into existing modules — cmd_* parsers next to their src/cli/ implementations, exec_tests toward the runner, the *_or_exit validators somewhere shared.
    2. A follow-up question raised separately: renaming main.sh to index.sh. Recommendation is no — index.sh currently means exactly one thing, a pure aggregator with only source lines and comments, enforced by test_module_aggregators_hold_only_source_lines_and_comments. A root index.sh holding 1427 lines of flag parsing would make the name mean two things depending on depth, and that test's glob (src/*/index.sh) would not even catch the drift. If src/ ending up with zero loose files is the goal, option 1 above achieves it without overloading the name.
  2. Chemaclass commented on Aug 1, 2026

    @Chemaclass
    MemberAuthor

    Decided: no rename. main.sh keeps its name; index.sh stays reserved for pure aggregators.

  3. Chemaclass commented on Aug 1, 2026

    @Chemaclass
    MemberAuthor

    Responsibility map

    Generated on dda105b: 1427 lines, 24 functions, zero file-scope statements — which makes
    this far safer than its size suggests. Nothing executes at source time.

    Decision on the open question

    cmd_* parsers stay together as one parsing layer, they do not move next to their
    src/cli/ implementations. Moving bashunit::main::cmd_doc into src/cli/doc.sh would put two
    namespaces in one file, and the alternative — renaming it — is forbidden here. The parsing
    layer is a real thing: it owns flag validation and the _or_exit helpers that every parser
    shares.

    Proposed layout — 8 files

    File Contents Lines
    main/index.sh aggregator —
    main/validate.sh abort_unknown_option, the three *_or_exit helpers, set_shard_or_exit 135
    main/test.sh cmd_test 485
    main/bench.sh cmd_bench 108
    main/subcommands.sh cmd_doc, cmd_init, cmd_learn, cmd_watch, cmd_upgrade 156
    main/assert.sh cmd_assert, exec_assert, exec_multi_assert, handle_assert_exit_code, is_assertion_function, is_exit_code_assertion 213
    main/watch.sh watch_loop, watch_get_checksum 73
    main/run.sh exec_tests, exec_benchmarks, cleanup, handle_stop_on_failure_sync 251

    cmd_test stays one file: it is a single while/case over ~60 flags, and the issue rightly
    forbids cutting a case across files. One 485-line file holding one function is navigable in a
    way a 1427-line file holding 24 is not.

    Layering, from all 19 internal call edges

    validate  <-  run  <-  watch
                     \      |
    assert (self-contained) |
    subcommands (leaves)    |
                    bench <-+  test
    

    test calls validate, run, watch and assert; bench calls validate and run; watch_loop calls
    exec_tests; assert and subcommands are self-contained.

    Two contract tests grep this file and must move with it

    • tests/unit/completions_test.sh:13 derives the test-subcommand flag list from
      cmd_test's parse loop (awk '/# Parse test-specific options/,/^ done$/') — the
      completions anti-drift contract.
    • tests/unit/completions_test.sh:23 derives the doc flag list from cmd_doc.

    Both will be repointed and mutation-tested, not just repointed. #946 shipped a layering
    contract that passed while checking nothing because its grep target had moved.

    Proceeding.

  4. Chemaclass commented on Aug 1, 2026

    @Chemaclass
    MemberAuthor

    Done in #955 (merged). src/ now holds 17 modules and zero flat files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactoringRefactoring or cleaning related

    Type

    No type

    Projects

    • Status
      Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions