summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-06-17 21:24:45 +0300
committerPaul Buetow <paul@buetow.org>2026-06-17 21:24:45 +0300
commit6109e2a6f5bc45c3c4080259bd09a1e3dc48ea4d (patch)
treedd862d7319be1c6a8b8ac24d15cd3518bd192c4e
parent5ebee8ef2a54b8578daf47b17fa82fc85b2cd1cd (diff)
on0 remove run_action_body serialization runner
Shuriken is a single-process CLI, yet the action layer could serialize 30+ globals plus every function definition (declare -p / declare -f) and pipe them into a fresh "bash -euo pipefail" process to run an action. Production already forced the in-process run_action_body_direct via SHURIKEN_ACTION_BODY_RUNNER, so the serialized-subprocess path was dead in production and only added complexity (a hand-maintained variable list to keep in sync). Per KISS, drop it. - Remove run_action_body_context and the run_action_body dispatcher. - Collapse run_configured_action_body to call the action in-process directly and remove the SHURIKEN_ACTION_BODY_RUNNER indirection in main(). - Move the only genuinely needed isolation into a test-only shim (test::run_action_isolated in tests/helpers.sh) for the generate real-failure test, which must capture a failure status without the in-process errexit abort ending the caller (correct in production, where main runs under errexit). - Update the errexit/status-propagation tests to exercise the direct path. Template-engine serialization is unrelated and left untouched. just test, just shellcheck, just check-generated and git diff --check all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
-rwxr-xr-xbin/shuriken93
-rw-r--r--src/lib/action.source.sh91
-rwxr-xr-xsrc/shuriken.sh2
-rwxr-xr-xtests/cli.sh26
-rwxr-xr-xtests/helpers.sh17
5 files changed, 63 insertions, 166 deletions
diff --git a/bin/shuriken b/bin/shuriken
index fd922e9..979f06a 100755
--- a/bin/shuriken
+++ b/bin/shuriken
@@ -5380,89 +5380,20 @@ run_simple_action() {
esac
}
-run_action_body_context() {
- local name
- local -a variable_names=(
- VERSION
- DEFAULTRC
- PACKAGED_TEMPLATE_DIR
- PACKAGED_ASSET_DIR
- DEFAULT_TEMPLATE_DIR
- DEFAULT_ASSET_DIR
- SHURIKEN_SOURCE_DIR
- SHURIKEN_OUTPUT_MODE
- SHURIKEN_ACTIVE_GENERATION_PID
- SHURIKEN_FORCE_GENERATE
- SHURIKEN_CURRENT_DATE_TEXT
- SHURIKEN_CONFIG_SOURCE
- SHURIKEN_FINAL_DIST_DIR
- INCOMING_DIR
- DIST_DIR
- TEMPLATE_DIR
- FAVICON
- TITLE
- HEIGHT
- THUMBHEIGHT
- MAXPREVIEWS
- IMAGE_JOBS
- IMAGEMAGICK_TIMEOUT
- TAR_TIMEOUT
- ORIGINAL_BASEPATH
- RANDOM_SEED
- SHUFFLE
- SPLASH_PAGE
- STATS_PAGE
- TARBALL_INCLUDE
- TARBALL_SUFFIX
- TAR_OPTS
- SYNC_DELETE
- SYNC_DESTINATIONS
- TEMPLATE_RENDER_FIELD_SPECS
- )
-
- for name in "${variable_names[@]}"; do
- declare -p "$name" 2>/dev/null || true
- done
- declare -f
-}
-
-run_action_body() {
- local -r action_name="$1"; shift
- local -i status=0
-
- if {
- run_action_body_context
- printf '%q "$@"\n' "$action_name"
- } | bash -euo pipefail -s -- "$@"; then
- status=0
- else
- status=$?
- fi
-
- return "$status"
-}
-
-run_action_body_direct() {
- "$@"
-}
-
+# Runs an action function in-process and propagates its exit status.
+#
+# Shuriken is a single-process CLI, so the action body runs as a plain function
+# call in the current shell. An earlier version could serialize 30+ globals plus
+# every function definition and pipe them into a fresh "bash -euo pipefail"
+# subprocess for isolation. That added real complexity (a hand-maintained list
+# of variables to forward) for no benefit here: there is no second process to
+# isolate from and nothing the action needs protecting from. Per KISS we dropped
+# the subprocess runner and call the action directly. Tests that genuinely need
+# subprocess isolation provide their own shim in tests/helpers.sh.
run_configured_action_body() {
local -r action_name="$1"; shift
- local -r runner="${SHURIKEN_ACTION_BODY_RUNNER:-run_action_body}"
- local -i status=0
- if [ "$runner" = run_action_body_direct ]; then
- "$runner" "$action_name" "$@"
- return
- fi
-
- if "$runner" "$action_name" "$@"; then
- status=0
- else
- status=$?
- fi
-
- return "$status"
+ "$action_name" "$@"
}
load_configured_action() {
@@ -5677,7 +5608,7 @@ main() {
return "$status"
fi
- SHURIKEN_ACTION_BODY_RUNNER=run_action_body_direct run_action
+ run_action
status=$?
if (( status != 0 )); then
return "$status"
diff --git a/src/lib/action.source.sh b/src/lib/action.source.sh
index edececa..425e009 100644
--- a/src/lib/action.source.sh
+++ b/src/lib/action.source.sh
@@ -23,89 +23,20 @@ run_simple_action() {
esac
}
-run_action_body_context() {
- local name
- local -a variable_names=(
- VERSION
- DEFAULTRC
- PACKAGED_TEMPLATE_DIR
- PACKAGED_ASSET_DIR
- DEFAULT_TEMPLATE_DIR
- DEFAULT_ASSET_DIR
- SHURIKEN_SOURCE_DIR
- SHURIKEN_OUTPUT_MODE
- SHURIKEN_ACTIVE_GENERATION_PID
- SHURIKEN_FORCE_GENERATE
- SHURIKEN_CURRENT_DATE_TEXT
- SHURIKEN_CONFIG_SOURCE
- SHURIKEN_FINAL_DIST_DIR
- INCOMING_DIR
- DIST_DIR
- TEMPLATE_DIR
- FAVICON
- TITLE
- HEIGHT
- THUMBHEIGHT
- MAXPREVIEWS
- IMAGE_JOBS
- IMAGEMAGICK_TIMEOUT
- TAR_TIMEOUT
- ORIGINAL_BASEPATH
- RANDOM_SEED
- SHUFFLE
- SPLASH_PAGE
- STATS_PAGE
- TARBALL_INCLUDE
- TARBALL_SUFFIX
- TAR_OPTS
- SYNC_DELETE
- SYNC_DESTINATIONS
- TEMPLATE_RENDER_FIELD_SPECS
- )
-
- for name in "${variable_names[@]}"; do
- declare -p "$name" 2>/dev/null || true
- done
- declare -f
-}
-
-run_action_body() {
- local -r action_name="$1"; shift
- local -i status=0
-
- if {
- run_action_body_context
- printf '%q "$@"\n' "$action_name"
- } | bash -euo pipefail -s -- "$@"; then
- status=0
- else
- status=$?
- fi
-
- return "$status"
-}
-
-run_action_body_direct() {
- "$@"
-}
-
+# Runs an action function in-process and propagates its exit status.
+#
+# Shuriken is a single-process CLI, so the action body runs as a plain function
+# call in the current shell. An earlier version could serialize 30+ globals plus
+# every function definition and pipe them into a fresh "bash -euo pipefail"
+# subprocess for isolation. That added real complexity (a hand-maintained list
+# of variables to forward) for no benefit here: there is no second process to
+# isolate from and nothing the action needs protecting from. Per KISS we dropped
+# the subprocess runner and call the action directly. Tests that genuinely need
+# subprocess isolation provide their own shim in tests/helpers.sh.
run_configured_action_body() {
local -r action_name="$1"; shift
- local -r runner="${SHURIKEN_ACTION_BODY_RUNNER:-run_action_body}"
- local -i status=0
-
- if [ "$runner" = run_action_body_direct ]; then
- "$runner" "$action_name" "$@"
- return
- fi
-
- if "$runner" "$action_name" "$@"; then
- status=0
- else
- status=$?
- fi
- return "$status"
+ "$action_name" "$@"
}
load_configured_action() {
diff --git a/src/shuriken.sh b/src/shuriken.sh
index 4114806..75f79cb 100755
--- a/src/shuriken.sh
+++ b/src/shuriken.sh
@@ -149,7 +149,7 @@ main() {
return "$status"
fi
- SHURIKEN_ACTION_BODY_RUNNER=run_action_body_direct run_action
+ run_action
status=$?
if (( status != 0 )); then
return "$status"
diff --git a/tests/cli.sh b/tests/cli.sh
index a28c1f1..2ca65ce 100755
--- a/tests/cli.sh
+++ b/tests/cli.sh
@@ -2428,7 +2428,10 @@ SHURIKEN_CLI_OVERRIDES=()
SHURIKEN_CLI_SYNC_DESTINATIONS=()
SHURIKEN_FORCE_GENERATE=no
-set +e
+# Production runs the action in-process under "set -euo pipefail" (main calls
+# run_action with errexit active), so an internal failure inside the action body
+# aborts immediately. Leave errexit ON here (no "set +e") to exercise that real
+# path: dry_run's "false" must abort before the "dry_run continued" write.
run_configured_action
BASH
)
@@ -2479,8 +2482,12 @@ export continued_file success_file
# shellcheck source=/dev/null
source <(sed '$d' "$shuriken")
+# The action runs in-process; a failing action must return a non-zero status so
+# a status-tested caller (the "if" below) takes the failure branch. "return 1"
+# fails honestly regardless of the caller's errexit state, and the write after
+# it must never run.
dry_run() {
- false
+ return 1
printf 'dry_run continued\n' > "$continued_file"
}
@@ -2544,8 +2551,12 @@ export continued_file success_file
# shellcheck source=/dev/null
source <(sed '$d' "$shuriken")
+# The action runs in-process; a failing action must return a non-zero status so
+# a status-tested caller (the "if" below) takes the failure branch. "return 1"
+# fails honestly regardless of the caller's errexit state, and the write after
+# it must never run.
dry_run() {
- false
+ return 1
printf 'dry_run continued\n' > "$continued_file"
}
@@ -2601,15 +2612,19 @@ test_generate_real_failure_returns_with_errexit_disabled() {
"$config_file" \
"$fake_bin" \
"$status_file" \
+ "$TEST_REPO_ROOT/tests/helpers.sh" \
2>&1 \
<<'BASH'
shuriken="$1"; shift
config_file="$1"; shift
fake_bin="$1"; shift
status_file="$1"; shift
+helpers="$1"; shift
# shellcheck source=/dev/null
source <(sed '$d' "$shuriken")
+# shellcheck source=/dev/null
+source "$helpers"
PATH="$fake_bin:$PATH"
SHURIKEN_CLI_ACTION=--generate
@@ -2619,8 +2634,11 @@ SHURIKEN_CLI_OVERRIDES=()
SHURIKEN_CLI_SYNC_DESTINATIONS=()
SHURIKEN_FORCE_GENERATE=no
+# A real generation failure aborts the shell in production (main runs under
+# errexit, by design). To capture the failure status and keep asserting, run the
+# action in the test-only isolation shim. The status it returns must be nonzero.
set +e
-run_configured_action
+test::run_action_isolated run_configured_action
generate_status=$?
printf '%s\n' "$generate_status" > "$status_file"
exit 0
diff --git a/tests/helpers.sh b/tests/helpers.sh
index 477acfc..d258c79 100755
--- a/tests/helpers.sh
+++ b/tests/helpers.sh
@@ -39,6 +39,23 @@ test::run_case() {
rm -f "$output_file"
}
+# Runs a shuriken action body in an isolated subshell and returns its status.
+#
+# Production runs action bodies in-process (see run_configured_action_body in
+# src/lib/action.source.sh). A few action bodies (notably generate_staged) flip
+# errexit back on internally and "return" a non-zero status while errexit is
+# active, which aborts the *caller's* shell on failure. In production that is
+# correct: main runs under "set -euo pipefail" and a generation failure should
+# end the process. A handful of tests, however, need to capture that failure
+# status and keep asserting afterwards. They used to get isolation for free from
+# the old serialized-subprocess runner; now that production runs in-process, this
+# test-only shim provides the minimal "run it where an abort can't kill me"
+# wrapper. The subshell catches any errexit/trap-driven abort, so the caller can
+# read the status with "$?".
+test::run_action_isolated() {
+ ( "$@" )
+}
+
test::assert_failure() {
local -r description="$1"; shift
local output_file