diff --git a/CHANGELOG.md b/CHANGELOG.md index bf3ce97..24a32cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), **Upgrading: no `task-init` re-run needed.** All seven installers already symlink `$SCRIPT_DIR/../bin/task-done` and never their own copy — the drift-guarded `install-shared-symlinks` stanza — so no installer-written artifact changes and no repo needs refreshing. ### Fixed +- **`task-remove` and the removal phase of `task-config set` no longer hang at 100% CPU on a large `CLAUDE.md` under bash 3.2 with a UTF-8 locale (#266).** Two "is anything left?" checks in `lib/loadout-artifacts.sh` stripped whitespace with `${s//[[:space:]]/}`: `_la_strip_line`, which decides whether `CLAUDE.md` (or `.gitignore`) held only task-init's line, and `_la_workflow_doc_tail`, which decides whether the workflow doc needs a `.bak`. Bash 3.2's pattern matcher is superlinear on a bracket class under a multibyte locale. On macOS's `/bin/bash` an 8KB UTF-8 file took 25s, and a ~30KB `CLAUDE.md` never finished. Both now go through a new `_la_has_content`, a `grep -q '[^[:space:]]'` on a here-string. It is not a pipe, because under the callers' `pipefail` a `grep -q` that exits early would SIGPIPE the writer and a non-blank file would read as blank, so the user's `CLAUDE.md` would be deleted. `_la_workflow_doc_tail` now returns the tail verbatim, not whitespace-stripped. Its only caller tests it for emptiness. The repo has no other `${var//[[:class:]]/}` uses. New tests drive both functions on a ~40KB UTF-8 file under a UTF-8 locale with a 20s alarm, and pin the whitespace-only cases. On the old code both large-file tests hit the alarm, and forcing `_la_has_content` to either answer turned tests red. The large-file tests only discriminate on bash 3.2; elsewhere they pin behaviour. **Upgrading: no `task-init` re-run needed.** `lib/` takes effect through the existing `~/.local/bin` symlinks. Workaround on an older checkout: `LC_ALL=C task-remove …`. + - **kiro-cli's V3 engine holds a dispatched worker's first turn with an agent-config upgrade dialog (#257): the launchers now warn.** With the V3 engine selected, kiro-cli opens "Your agent configs are still in the V2 format. Upgrade them to run on V3?" before the positional first turn. Observed on kiro-cli 2.32.0 on the exact launch line `task-work` builds, `--tui` included. Today V3 means `chat.agentEngine = v3`. kiro-cli's help calls v2 "the pre-3.0 default", so V3 is expected to become the default with 3.0. What the probes established, each in a pty against a real kiro-cli 2.32.0: - **The dialog is a setting, `chat.enableAutoAgentUpgrade`.** When it is unset kiro-cli prompts, `true` upgrades silently and `false` never asks. With `false` set (workspace-scoped for the probe), `--v3` showed no dialog, the positional first turn ran, and all three hooks fired in the task-init (V2) shape, agentSpawn, userPromptSubmit and stop. The config file was not touched. kiro-cli has no flag or env var that sets this per launch, so it cannot be pinned on the launch line the way `--tui` was. - **Shipping upgraded configs does not avoid the dialog.** A probe agent upgraded with `kiro-cli agent upgrade` still got it under `--v3`. That is probably because the prompt covers every config kiro-cli discovers, global ones included, but this was not isolated. diff --git a/lib/loadout-artifacts.sh b/lib/loadout-artifacts.sh index 53bb586..00f5d57 100644 --- a/lib/loadout-artifacts.sh +++ b/lib/loadout-artifacts.sh @@ -380,7 +380,18 @@ _la_workflow_doc_tail() { stub=$(_managed_tail_stub) # Strip the stub wherever it sits, then see whether anything is left. tail=${tail//"$stub"/} - printf '%s' "${tail//[[:space:]]/}" + if _la_has_content "$tail"; then printf '%s' "$tail"; fi +} + +# _la_has_content +# True when holds anything besides whitespace. Not +# `[[ -n "${s//[[:space:]]/}" ]]`: bash 3.2's matcher is superlinear on a +# bracket class under a multibyte locale, so over a file-sized string that +# substitution never finishes (#266). A here-string rather than a pipe, because +# `grep -q` exits on its first match and under the callers' pipefail the +# SIGPIPE'd printf would make a non-blank file read as blank. +_la_has_content() { + grep -q '[^[:space:]]' <<<"$1" } # Everything in tasks/ that is backlog rather than task-init scaffolding, one @@ -596,7 +607,7 @@ _la_strip_line() { local remaining remaining=$(grep -vxF -- "$line" "$f" || true) - if [[ -z "${remaining//[[:space:]]/}" ]]; then + if ! _la_has_content "$remaining"; then if [[ "$mode" == apply ]]; then rm -f "$f"; fi if [[ "$mode" == plan ]]; then printf ' would remove %s (it held only the %s)\n' "$rel" "$what" diff --git a/tests/task_remove.bats b/tests/task_remove.bats index 30a049f..098bffd 100644 --- a/tests/task_remove.bats +++ b/tests/task_remove.bats @@ -178,6 +178,89 @@ _assert_clean_removal() { assert_success } +# #266: the "is anything left?" checks used `${s//[[:space:]]/}`, which bash +# 3.2's matcher runs superlinearly under a multibyte locale — an 8KB UTF-8 file +# took 25s, a 30KB CLAUDE.md never finished. Drive the two functions directly in +# their own bash, under a UTF-8 locale, with an alarm: a hang then kills that +# bash rather than an orphaned subshell of task-remove spinning on after the +# test. Only bites on bash 3.2 (macOS's /bin/bash); elsewhere these pass either +# way and still pin the behaviour. +_utf8_locale() { + locale -a 2>/dev/null | grep -iE '^(en_US|C)\.utf-?8$' | head -1 +} + +# ~40KB of the user's own text on stdout, non-ASCII on every line. Callers +# redirect it: a `> /dev/stdout` inside would reopen, and on Linux truncate, +# whatever file the caller's stdout is. +_big_utf8_text() { + local i + for ((i = 0; i < 1000; i++)); do + printf 'línea número %d — café ✓ notes of the house\n' "$i" + done +} + +_run_with_deadline() { + local loc="$1"; shift + run env LC_ALL="$loc" perl -e 'alarm 20; exec @ARGV' \ + bash -c ". '$REPO_ROOT_REAL/lib/loadout-artifacts.sh'; $*" +} + +@test "a large UTF-8 CLAUDE.md is stripped promptly under a UTF-8 locale (#266)" { + local loc; loc=$(_utf8_locale) + [[ -n "$loc" ]] || skip "no UTF-8 locale installed" + _big_utf8_text > "$MAIN_REPO/CLAUDE.md" + printf '@.claude/gh-workflow.md\n' >> "$MAIN_REPO/CLAUDE.md" + + _run_with_deadline "$loc" "_la_strip_line '$MAIN_REPO' CLAUDE.md '@.claude/gh-workflow.md' apply import" + assert_failure 1 # 1 = stripped, file kept; 142 would be the alarm + assert_output --partial "stripped the import from CLAUDE.md" + run grep -qxF '@.claude/gh-workflow.md' "$MAIN_REPO/CLAUDE.md" + assert_failure + run grep -cF 'café' "$MAIN_REPO/CLAUDE.md" + assert_output 1000 +} + +@test "a CLAUDE.md left holding only whitespace is still removed (#266)" { + printf '\n \t\n@.claude/gh-workflow.md\n\n' > "$MAIN_REPO/CLAUDE.md" + run _la_strip_line "$MAIN_REPO" CLAUDE.md '@.claude/gh-workflow.md' apply import + assert_success + assert_output --partial "removed CLAUDE.md (it held only the import)" + assert [ ! -e "$MAIN_REPO/CLAUDE.md" ] +} + +@test "a large UTF-8 workflow-doc tail is read promptly under a UTF-8 locale (#266)" { + local loc; loc=$(_utf8_locale) + [[ -n "$loc" ]] || skip "no UTF-8 locale installed" + local doc="$MAIN_REPO/doc.md" + { + printf '%s\n' '' 'body' '' + _big_utf8_text + } > "$doc" + + # Redirected, not piped: a pipeline would run the function in a subshell the + # alarm does not reach. + _run_with_deadline "$loc" "_la_workflow_doc_tail '$doc' > '$MAIN_REPO/tail.out'" + assert_success + run grep -cF 'café' "$MAIN_REPO/tail.out" + assert_output 1000 +} + +@test "a workflow-doc tail of only the stub and whitespace counts as empty (#266)" { + local doc="$MAIN_REPO/doc.md" + { + printf '%s\n' '' 'body' '' '' ' ' + _managed_tail_stub + printf '\t\n\n' + } > "$doc" + run _la_workflow_doc_tail "$doc" + assert_success + assert_output "" + + printf 'Green here means ./run_tests.sh.\n' >> "$doc" + run _la_workflow_doc_tail "$doc" + assert_output --partial "Green here means" +} + @test "a settings.json carrying a non-radio hook keeps that hook and the file" { mkdir -p "$MAIN_REPO/.claude" cat > "$MAIN_REPO/.claude/settings.json" <<'JSON'