diff --git a/docs/superpowers/evidence/closing-brace-continuation-2026-08-22.json b/docs/superpowers/evidence/closing-brace-continuation-2026-08-22.json new file mode 100644 index 00000000..3ccd3cb9 --- /dev/null +++ b/docs/superpowers/evidence/closing-brace-continuation-2026-08-22.json @@ -0,0 +1,44 @@ +{ + "intent": "Teach continuation_lines() that a line holding only a group's closing brace plus an operator (`} |`, `} >>file`) is not a statement, so kcov stops inflating the denominator with it. This is defect (a) in docs/kcov-under-report.md, in the flavour that caps four fixes/lib libs.", + "scope": [ + "meta/scripts/lib/shell_coverage_lines.py (one regex and one branch)", + "python_pkg/tests/test_shell_coverage_lines.py (four new cases: two positive, two negative)", + "Non-goal: the backslash-continued ASSIGNMENT flavour that caps bt_adapter.sh line 97 -- it needs a command-vs-assignment distinction and is one line in one lib", + "Non-goal: clearing any lib from meta/shell-coverage-allowlist.txt, which follows in its own commit once each is re-measured" + ], + "changes": [ + "Added _CLOSING_BRACE_OP = ^\\s*\\}\\s*(\\||>>?|<|&>) and one branch in continuation_lines() that excludes matching lines when no quote is open.", + "Anchoring on a LEADING closing brace is what makes this safe. bash attributes each command inside a group to its own line, and a pipe's right-hand side to its own line, so the brace-plus-operator line is left with no statement. A one-liner `{ echo a; } >file` starts with `{` and does not match; a bare `}` closing a function carries no operator and does not match.", + "Added four tests: `} |` and `} >>\"$REPORT_FILE\"` are continuations; a bare `}` closing a function and a single-line `{ echo a; } >\"$out\"` are NOT. The negative pair is the important half, because over-detection shrinks the denominator and INFLATES coverage -- the fail-open direction this test module's own header calls out." + ], + "verification": [ + { + "command": "python3 -m coverage run --branch --source=meta/scripts/lib -m pytest python_pkg/tests/test_shell_coverage_lines.py", + "result": "pass", + "evidence": "18 passed. shell_coverage_lines.py: 56 statements, 40 branches, 0 missed, 0 partial = 100% -- so the new branch is covered in both directions, as the repo's fail_under=100 branch gate requires." + }, + { + "command": "Audit every line the new predicate newly excludes across the whole allowlist plus the cleared libs", + "result": "pass", + "evidence": "17 lines excluded across 10 libs, and every one re-matched the brace-plus-operator shape when printed with its text: integrity.sh:68/118, aw_autostart.sh:65, rpi_nc_ssl.sh:88, wg_sshd.sh:66, arch_perf_probes.sh:46/93, arch_perf_report.sh:20, bt_audio.sh:90/113/137/145/157, nvidia_config.sh:68, trace_prefix.sh:100, trace_stubs.sh:69, adb_trusted.sh:29. No ordinary statement was caught." + }, + { + "command": "bash meta/scripts/shell_coverage_jail.sh --subject .../fixes/lib/tests/run_all.sh --measure --min 1 --fail-on-case-error -- \"\", for five libs", + "result": "pass", + "evidence": "bt_audio.sh 96.97% -> 100.00% (127/127), arch_perf_report.sh 98.33% -> 100.00% (59/59), bt_report.sh 100.00% (37/37) unchanged, bt_pairing.sh 100.00% (65/65) unchanged, bt_adapter.sh 98.55% (68/69) unchanged. Two libs gained, three held, none went DOWN -- the handoff's stated regression signal." + }, + { + "command": "The handoff's acceptance test: dot_resolver_install.sh and rpi_nc_ca.sh through features/lib/tests/run_all.sh with its jail_args", + "result": "pass", + "evidence": "dot_resolver_install.sh: 39/39 = 100.00% and rpi_nc_ca.sh: 73/73 = 100.00%, exactly the figures the handoff names as the instrument's acceptance criteria." + } + ], + "risks": [ + "A denominator change touches every measured lib, which is why it lands alone. The arithmetic bounds the blast radius: _measure computes denominator = kcov's line set minus excluded, and covered_set = (executed | traced) & denominator, so for any lib already at 100.00% the covered set IS the denominator and removing a subset leaves it at 100.00%. The five gated pacman_hook_stall libs, bt_report and bt_pairing therefore cannot regress, which the measurements above confirm for the two that were re-run.", + "The predicate is deliberately narrow. It does not cover `} &&`, `} ||` or a closing brace followed by a comment, and it does not touch the backslash-continued assignment flavour. Each would need its own evidence that bash attributes no statement to the line." + ], + "rollback": [ + "git revert the commit; continuation_lines() returns to quote-state detection only and the four new tests go with it.", + "After rollback, re-measure bt_audio.sh and arch_perf_report.sh and expect them back at 96.97% and 98.33%; neither is on the allowlist-cleared list, so no gate breaks." + ] +} diff --git a/meta/scripts/lib/shell_coverage_lines.py b/meta/scripts/lib/shell_coverage_lines.py index af4c2e5c..7b7aa896 100755 --- a/meta/scripts/lib/shell_coverage_lines.py +++ b/meta/scripts/lib/shell_coverage_lines.py @@ -66,6 +66,16 @@ def traced_paths(trace_dir: Path, subject: str) -> list[Path]: return found +# A line holding only a group's closing brace plus an operator: `} |`, +# `} >>file`, `} log`. kcov instruments these, but bash attributes +# no statement to them -- each command INSIDE the group is attributed to its +# own line, and a pipe's right-hand side to its own line, leaving the brace +# line with nothing. Anchoring on a leading `}` is what keeps this safe: a +# one-liner `{ echo a; } >file` begins with `{` and is a real statement, and a +# bare `}` closing a function carries no operator. +_CLOSING_BRACE_OP = re.compile(r"^\s*\}\s*(\||>>?|<|&>)") + + def continuation_lines(source: Path) -> set[int]: """Return lines that are *inside* a multi-line quoted argument. @@ -75,9 +85,13 @@ def continuation_lines(source: Path) -> set[int]: them as executable at all, which is the correct reading: they are data inside an argument, not statements. - Detected by tracking quote state across the file: any line that *begins* - while a quote opened on an earlier line is still open is a continuation. - Only the opening line of such a construct is a statement. + Two shapes qualify: + + * a line that *begins* while a quote opened on an earlier line is still + open -- the continuation of a multi-line quoted argument. Only the + opening line of such a construct is a statement. + * a line holding just a group's closing brace and an operator (``} |``, + ``} >>file``). See ``_CLOSING_BRACE_OP``. """ inside: set[int] = set() quote: str | None = None @@ -86,24 +100,35 @@ def continuation_lines(source: Path) -> set[int]: ): if quote is not None: inside.add(number) - index = 0 - while index < len(raw): - char = raw[index] - if quote is None: - if char == "#": - # A comment outside quotes ends the line's significance. - break - if char == "\\": - index += 2 - continue - if char in ("'", '"'): - quote = char - elif char == quote: - quote = None - elif quote == '"' and char == "\\": - # Only double quotes honour backslash escapes; inside single - # quotes a backslash is a literal character. + quote = _quote_after(raw, quote) + if quote is None and _CLOSING_BRACE_OP.match(raw): + inside.add(number) + return inside + + +def _quote_after(raw: str, quote: str | None) -> str | None: + """Return the open-quote state at the end of ``raw``, given its state at the start. + + ``None`` means no quote is open. Escapes are honoured outside quotes and + inside double quotes, but not inside single quotes, which take a + backslash literally. + """ + index = 0 + while index < len(raw): + char = raw[index] + if quote is None: + if char == "#": + # A comment outside quotes ends the line's significance. + break + if char == "\\": index += 2 continue - index += 1 - return inside + if char in ("'", '"'): + quote = char + elif char == quote: + quote = None + elif quote == '"' and char == "\\": + index += 2 + continue + index += 1 + return quote diff --git a/python_pkg/tests/test_shell_coverage_lines.py b/python_pkg/tests/test_shell_coverage_lines.py index 4ad72ab3..db82628e 100644 --- a/python_pkg/tests/test_shell_coverage_lines.py +++ b/python_pkg/tests/test_shell_coverage_lines.py @@ -79,6 +79,44 @@ def test_multiline_double_quote_marks_inner_lines( assert mod.continuation_lines(src) == {2, 3} +def test_closing_brace_with_pipe_is_a_continuation( + mod: ModuleType, tmp_path: Path +) -> None: + """`} |` carries no statement: bash attributes each part to its own line.""" + src = _write( + tmp_path, + "a.sh", + "{\n\techo a\n\techo b\n} |\n\tconsumer\n", + ) + assert mod.continuation_lines(src) == {4} + + +def test_closing_brace_with_redirect_is_a_continuation( + mod: ModuleType, tmp_path: Path +) -> None: + """The arch_perf_report.sh shape: a group appended to a report file.""" + src = _write( + tmp_path, + "a.sh", + '{\n\techo a\n} >>"$REPORT_FILE"\necho after\n', + ) + assert mod.continuation_lines(src) == {3} + + +def test_bare_closing_brace_is_a_statement(mod: ModuleType, tmp_path: Path) -> None: + """A `}` closing a function has no operator and must stay in the count.""" + src = _write(tmp_path, "a.sh", "f() {\n\techo a\n}\nf\n") + assert mod.continuation_lines(src) == set() + + +def test_single_line_group_with_redirect_is_a_statement( + mod: ModuleType, tmp_path: Path +) -> None: + """`{ echo a; } >file` begins with `{`, so the whole line is a statement.""" + src = _write(tmp_path, "a.sh", '{ echo a; } >"$out"\necho after\n') + assert mod.continuation_lines(src) == set() + + def test_comment_outside_quotes_ends_the_line(mod: ModuleType, tmp_path: Path) -> None: """An apostrophe inside a comment must not open a quote state.""" src = _write(tmp_path, "a.sh", "# don't be fooled\necho after\n")