From ce139b58673192cb29aab54d9997b2e00057f9fd Mon Sep 17 00:00:00 2001
From: Krzysztof kuhy Rudnicki <krzysztofrudnicki0@gmail.com>
Date: Mon, 17 Aug 2026 15:15:25 +0200
Subject: [PATCH] extract_shell_functions: never leave a function defined twice

On a script with no `set -e` anchor -- every #!/system/bin/sh phone
enforcer -- the tool wrote the library, then exited on the missing
anchor, leaving the functions in the entry script as well. The result
defined each moved function twice, lints clean, and passes a
function-body hash check, so nothing downstream catches it. It cost a
split of dns_enforcer.sh this session: 325 lines in, 331 lines out.

Both writes now happen after the anchor search. A missing anchor is a
warning naming the exact line to paste, not an exit, so the move still
completes. Added a shrink guard that refuses to write when the entry
script would not get shorter, and an explicit error when the selected
function set is empty.

Verified both paths: dns_enforcer.sh (no anchor) warns and still drops
171 -> 158 with the functions gone, phone_backup.sh (anchored) drops
333 -> 264 with the source line auto-inserted as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
---
 ...tract-tool-duplication-fix-2026-08-17.json | 42 +++++++++++++++++++
 meta/scripts/extract_shell_functions.py       | 36 +++++++++++++---
 2 files changed, 73 insertions(+), 5 deletions(-)
 create mode 100644 docs/superpowers/evidence/extract-tool-duplication-fix-2026-08-17.json

diff --git a/docs/superpowers/evidence/extract-tool-duplication-fix-2026-08-17.json b/docs/superpowers/evidence/extract-tool-duplication-fix-2026-08-17.json
new file mode 100644
index 00000000..7b7278dc
--- /dev/null
+++ b/docs/superpowers/evidence/extract-tool-duplication-fix-2026-08-17.json
@@ -0,0 +1,42 @@
+{
+  "intent": "Stop extract_shell_functions.py from producing a split in which every moved function is defined twice, which is what it did on any script lacking a `set -e` anchor.",
+  "scope": [
+    "meta/scripts/extract_shell_functions.py",
+    "Non-goal: changing how functions are located or how the source line is worded when the anchor IS found."
+  ],
+  "changes": [
+    "Moved both file writes to after the anchor search. The library was previously written before the anchor check and the process then exited, so the functions stayed in the entry script AND appeared in the new library.",
+    "A missing anchor is now a warning on stderr naming the exact two lines to paste by hand, not an exit. The move still completes, so the operator's remaining job is one paste rather than an undo.",
+    "Added a shrink guard: if the rewritten entry script would not be shorter than the original, the tool refuses to write anything. A duplicate-definition split lints clean and passes a function-body hash check, so nothing downstream would have caught it.",
+    "Added an explicit failure when the selected function set is empty, rather than writing an empty library."
+  ],
+  "verification": [
+    {
+      "command": "python3 meta/scripts/extract_shell_functions.py <copy of dns_enforcer.sh> lg.sh --functions log,rotate_log",
+      "result": "pass",
+      "evidence": "Prints the warning naming `. \"$SCRIPT_DIR/lg.sh\"`, and the entry script drops 171 -> 158 lines with `grep -c '^log()|^rotate_log()'` returning 0. Before this change the same command left both functions in place and the file unchanged in length."
+    },
+    {
+      "command": "python3 meta/scripts/extract_shell_functions.py <copy of phone_backup.sh> pb_lib.sh --functions usage,verify_backup,dump_packages",
+      "result": "pass",
+      "evidence": "333 -> 264 lines, library 77 lines, and the source directive is auto-inserted at line 40 after `set -euo pipefail`. The anchored path is unchanged by this fix."
+    },
+    {
+      "command": "python3 meta/scripts/extract_shell_functions.py <script> none.sh --functions nonexistent_fn",
+      "result": "fail-as-expected",
+      "evidence": "'no such top-level function(s): nonexistent_fn' and no library is written."
+    },
+    {
+      "command": "pre-commit run --files meta/scripts/extract_shell_functions.py",
+      "result": "pass",
+      "evidence": "All applicable hooks pass, including ruff, mypy, pylint and bandit."
+    }
+  ],
+  "risks": [
+    "The warning is easy to miss in a long transcript, and a split left unsourced is a script whose functions have silently vanished. The shrink guard does not catch this case, because the file genuinely did shrink. Running the entry script, as this session does after each split, is what catches it."
+  ],
+  "rollback": [
+    "git revert the commit. The tool returns to writing the library before the anchor check.",
+    "After rollback, re-run the dns_enforcer.sh case above and confirm the duplicate definitions reappear, which is the behaviour this commit removes."
+  ]
+}
diff --git a/meta/scripts/extract_shell_functions.py b/meta/scripts/extract_shell_functions.py
index 0e0ccd9f..d43f8621 100755
--- a/meta/scripts/extract_shell_functions.py
+++ b/meta/scripts/extract_shell_functions.py
@@ -45,12 +45,15 @@ def split_script(
 ) -> tuple[int, int]:
     """Move the named functions (or all of them) into *lib_path*."""
     lines = path.read_text().split("\n")
+    before_len = len(lines)
     blocks = find_function_blocks(lines)
     if names:
         blocks = [b for b in blocks if b[2] in names]
         missing = set(names) - {b[2] for b in blocks}
         if missing:
             sys.exit(f"no such top-level function(s): {', '.join(sorted(missing))}")
+    if not blocks:
+        sys.exit(f"no top-level functions to move out of {path}")
 
     parts: list[str] = []
     drop: set[int] = set()
@@ -61,9 +64,6 @@ def split_script(
         parts.append("\n".join(lines[head : end + 1]))
         drop.update(range(head, end + 1))
 
-    lib_path.parent.mkdir(parents=True, exist_ok=True)
-    lib_path.write_text(header.rstrip() + "\n\n" + "\n\n".join(parts) + "\n")
-
     kept = [ln for n, ln in enumerate(lines) if n not in drop]
     rel = lib_path.relative_to(path.parent)
     source_line = '. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/' + str(rel) + '"'
@@ -75,9 +75,35 @@ def split_script(
         if not inserted and re.match(r"^set -[euo pipefail]+\b", line.strip()):
             out += ["", f"# shellcheck source={rel}", source_line]
             inserted = True
-    if not inserted:
-        sys.exit("no `set -e` line found to anchor the source directive")
+
+    # Both files are written only once the whole split is known to be
+    # expressible. Writing the library before this point and then exiting on a
+    # missing anchor left the functions defined in BOTH files, so the entry
+    # script came out longer than it started -- a duplicate-definition split
+    # that still lints clean and still passes a function-body hash check.
+    # A split that does not shrink the entry script did not move anything; it
+    # duplicated. Refuse rather than write, since the result lints clean and
+    # passes a function-body hash check while defining every function twice.
+    if len(out) >= before_len:
+        sys.exit(
+            f"refusing to write: {path} would go from {before_len} to {len(out)} "
+            f"lines, so nothing was actually moved out of it"
+        )
+
+    lib_path.parent.mkdir(parents=True, exist_ok=True)
+    lib_path.write_text(header.rstrip() + "\n\n" + "\n\n".join(parts) + "\n")
     path.write_text("\n".join(out))
+
+    if not inserted:
+        sys.stderr.write(
+            f"warning: no `set -e` line in {path} to anchor the source directive.\n"
+            f"         The functions were moved and {lib_path.name} was written, "
+            f"but nothing sources it yet.\n"
+            f'         Add this by hand, after the existing `. "$SCRIPT_DIR/..."` '
+            f"line:\n"
+            f"             # shellcheck source={rel}\n"
+            f'             . "$SCRIPT_DIR/{rel}"\n'
+        )
     return len(out), len(lib_path.read_text().split("\n"))