claudiomaradonna/azkaban
1
1
Fork 0
Code Issues Pull requests Activity

refactor: simplify the launcher without changing behavior #50

Merged
claudiomaradonna merged 9 commits from refactor/simplify-launcher into master 2026-10-17 15:45:48 +02:00
Conversation 0 Commits 9 Files changed 8 +246 -230
motoko commented 2026-10-17 15:43:28 +02:00
Collaborator
Copy link

Why

A review of the launcher found the same logic copied in several places
and two functions over a hundred lines long, so a fix to one copy could
miss the others. This change reshapes that code without changing what
it does.

What changed

  • One helper, _azk_split_mount, parses mount values; it replaces five
    hand-written copies.
  • _azk_canonicalize_existing_path resolves the final parent folder in
    one place.
  • The env loader tracks keys it has already seen in an associative
    array.
  • _azk_main calls named steps for the unknown-AZKABAN_* warning and
    the container env lines.
  • Custom-image approval and the build are separate functions, and the
    pending copy is removed on one path.
  • lib/azk-host.sh names the engine (azk_host_engine_name), and the
    installer checks both engines' versions with one function.
  • Adapters use two new core helpers, _azk_check_state_dirs and
    _azk_create_state_dirs, documented in docs/adding-a-client.md.
    These add to the adapter contract without breaking it.

Behavior, error messages, --dry-run output and the adapter contract
are unchanged.

How to verify

Run make lint && .agents/hooks/shellcheck.sh repo && make test: it
passes, with 41 files ShellCheck-clean and 328 tests, 0 failures. No
existing test was changed. The first commit only adds assertions that
an approval leaves just the approval record; they pass on master.
Each commit passes lint and the tests. Not run: make test-e2e and
the macOS paths.

Considerations

One race-only difference in Claude's state setup: if config changes
in the instant after mkdir -p, the launch now stops with "Claude
state path '…/config' is not a directory", naming config. Before, it
failed when writing settings.json, or named the state folder. Either
way the launch stops safely.

## Why A review of the launcher found the same logic copied in several places and two functions over a hundred lines long, so a fix to one copy could miss the others. This change reshapes that code without changing what it does. ## What changed - One helper, `_azk_split_mount`, parses mount values; it replaces five hand-written copies. - `_azk_canonicalize_existing_path` resolves the final parent folder in one place. - The env loader tracks keys it has already seen in an associative array. - `_azk_main` calls named steps for the unknown-`AZKABAN_*` warning and the container env lines. - Custom-image approval and the build are separate functions, and the pending copy is removed on one path. - `lib/azk-host.sh` names the engine (`azk_host_engine_name`), and the installer checks both engines' versions with one function. - Adapters use two new core helpers, `_azk_check_state_dirs` and `_azk_create_state_dirs`, documented in `docs/adding-a-client.md`. These add to the adapter contract without breaking it. Behavior, error messages, `--dry-run` output and the adapter contract are unchanged. ## How to verify Run `make lint && .agents/hooks/shellcheck.sh repo && make test`: it passes, with 41 files ShellCheck-clean and 328 tests, 0 failures. No existing test was changed. The first commit only adds assertions that an approval leaves just the approval record; they pass on `master`. Each commit passes lint and the tests. Not run: `make test-e2e` and the macOS paths. ## Considerations One race-only difference in Claude's state setup: if `config` changes in the instant after `mkdir -p`, the launch now stops with "Claude state path '…/config' is not a directory", naming `config`. Before, it failed when writing `settings.json`, or named the state folder. Either way the launch stops safely. 
The approval and the unchanged-record launches now assert that the
approvals folder holds the record alone, so the per-launch pending
copy's cleanup stays pinned through the launcher refactor.
Five sites in bin/azkaban split a -v value into its source and
destination by hand; _azk_split_mount now does it for all of them.
_azk_canonicalize_existing_path resolved a path's folder in the link
loop and again after it; one loop now enters the folder physically
before each check of the last name.
_load_env_file checked each key against every key seen so far with a
linear scan; an associative array keyed by name now answers it.
_azk_main warned about unknown AZKABAN_ names and built the container
env lines inline; _azk_warn_unknown_controls and
_azk_collect_container_env now hold those steps.
_azk_select_custom_image copied, compared, prompted for, recorded, and
built a workspace Dockerfile in one function, removing its pending copy
on six exit paths.  _azk_approve_custom_image and
_azk_build_custom_image now hold those steps, and the caller removes
the pending copy once.
The launcher's run step and the installer's build step each chose
podman or container themselves; azk_host_engine_name now sets
AZK_HOST_ENGINE for them and for azk_host_engine.
The installer's Podman and Apple container version checks differed
only in the engine's name and minimum version;
_azk_install_check_engine_version now takes both.
Each adapter checked its state folders, created them privately, and
checked them again by hand.  The core's _azk_check_state_dirs and
_azk_create_state_dirs now carry those steps, taking the wording that
differs between adapters as arguments, so every diagnostic stays the
same on every path but one race: when Claude's config folder vanishes
or changes in the instant after mkdir -p, the launch now stops at the
post-mkdir check, naming config, where it stopped at the seed file or
named the state folder.  docs/adding-a-client.md describes the helpers
and that callers pass their status on.
claudiomaradonna deleted branch refactor/simplify-launcher 2026-10-17 15:45:52 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
claudiomaradonna/azkaban!50
No description provided.