fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: close three demonstrated bypasses of the comment cap, and self-test it

Browse Source 
Adversarial review of PR #44 constructed all three, each compiling clean with
no warnings, each passing the checker.

Block comments were invisible: the scanner only ever tested startswith("//"),
so a six-line /* */ passed — and /** */ is a DOC comment that rustdoc renders
exactly as /// does, making `/**` the cheapest way to write the long argument
the rule exists to forbid. There are zero block comments in the tree today,
which is the only reason this was unexploited; #44 is the change that gives
anyone a reason to look.

Fence state was per-FILE, so one unclosed ``` exempted every later comment in
the file. A dangling doctest would fail the build in a lib target, but doctests
do not run for bins, tests/, examples/ or harness/ — all default scan roots.
Fence state now belongs to the comment block: any non-comment line clears it.

Attributes terminated a run, so `#[inline]` between two pairs of doc lines
split one argument of four into two legal runs of two. The module docstring
had always promised attributes are "ignored"; the code disagreed with it, and
disagreed in the direction that passes.

Closing the last of those found two real violations the old checker could not
see. crates/improvise-command/src/command/cmd/text_buffer.rs carried TWO doc
comments straddling #[derive(Debug)] — both attach to the struct, so rustdoc
rendered both; a leftover from the comment wave. transport.rs had a three-line
run split by #[allow].

A checker with ten silent successes and no test of its own is the thing it
warns about, so --self-test runs every bypass this file has been fixed for
plus three shapes that must stay legal. Proved discriminating by reverting the
attribute fix: it fails and names that case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
2026-09-20 11:05:22 -07:00
parent 52507a6678
commit 53ffe9f193
4 changed files with 183 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.beads/issues.jsonl
+31 -23
View File
File diff suppressed because one or more lines are too long
crates/improvise-command/src/command/cmd/text_buffer.rs
+1 -2
View File
@@ -344,10 +344,9 @@ impl Cmd for CommandModeBackspace {
// ── Command mode execution ──────────────────────────────────────────────────
/// Execute the command in the "command" buffer (the `:` command line).
#[derive(Debug)]
/// Execute the `:` command buffer through the registry. The `:` prompt is another
/// front end to the scripting language — the same parser as `improvise script`.
#[derive(Debug)]
pub struct ExecuteCommand;
impl Cmd for ExecuteCommand {
fn name(&self) -> &'static str {
crates/improvise-ws-server/src/transport.rs
+2 -3
View File
@@ -111,10 +111,9 @@ fn attach(
}
}
// The callback's error type is tungstenite's, its size is not ours to reduce,
// and this callback never returns one: refusal is the registry's answer.
/// One websocket connection, from handshake to close. The large error type is
/// tungstenite's and is never returned: refusal is the registry's answer.
#[allow(clippy::result_large_err)]
/// One websocket connection, from handshake to close.
async fn connection(stream: TcpStream, tx: mpsc::UnboundedSender<Inbound>) -> Result<()> {
// The document is named in the URL, which is readable only during the
// handshake, so it is captured out of the callback.
scripts/check-comment-runs.py
+149
View File
@@ -39,8 +39,27 @@ def comment_runs(lines):
run_len = 0
in_fence = False # are we between an opening and a closing fence?
fence_exempt = False # is THIS fence's body a doctest rustdoc will run?
in_block = False # inside a /* ... */ or /** ... */ comment?
for n, raw in enumerate(lines, 1):
s = raw.strip()
# A block comment is a comment run like any other, and /** */ is a DOC
# comment: rustdoc renders it exactly as /// does (improvise-9c8c).
if in_block:
if run_start is None:
run_start, run_len = n, 0
run_len += 1
if "*/" in s:
in_block = False
continue
if s.startswith("/*"):
if run_start is None:
run_start, run_len = n, 0
run_len += 1
if "*/" not in s[2:]:
in_block = True
continue
is_comment = s.startswith("//")
if is_comment and s.lstrip("/!").strip().startswith("```"):
if in_fence:
@@ -71,6 +90,12 @@ def comment_runs(lines):
run_start, run_len = n, 0
run_len += 1
else:
# Fence state belongs to the comment block, not to the file: an
# unclosed ``` used to exempt every later comment in the file.
in_fence = False
fence_exempt = False
if _is_attribute(s):
continue
if run_start is not None and run_len > 0:
yield run_start, run_len
run_start, run_len = None, 0
@@ -90,6 +115,16 @@ def rust_files(roots):
_TEST_ATTRS = ("test", "tokio::test", "async_std::test", "wasm_bindgen_test")
def _is_attribute(line):
"""An attribute line: not a comment, and not a terminator either.
The module docstring has always promised attributes are ignored; before
this they went down the terminator branch, so `#[inline]` between two pairs
of doc lines split one run of four into two legal runs of two.
"""
return line.startswith("#[") or line.startswith("#![")
def _is_test_attribute(line):
"""True only for an attribute that actually marks a test.
@@ -178,6 +213,116 @@ def check_evidence(roots):
return 1
SELF_TEST_CASES = [
("a plain run of three", 3, """
// one
// two
// three
fn f() {}
"""),
("a block comment of six", 6, """
/* one
two
three
four
five
*/
fn f() {}
"""),
("a rustdoc block comment of six", 6, """
/** one
* two
* three
* four
* five
*/
fn f() {}
"""),
("an unclosed fence must not exempt the rest of the file", 6, """
/// opens a fence and never closes it
/// ```
/// let x = 1;
fn a() {}
/// one
/// two
/// three
/// four
/// five
/// six
fn b() {}
"""),
("an attribute must not split a run", 4, """
/// one
/// two
#[inline]
/// three
/// four
fn c() {}
"""),
("a non-runnable fence is prose and counts", 4, """
/// one
/// ```text
/// two
/// ```
fn d() {}
"""),
]
SELF_TEST_CLEAN = [
("two lines is the limit", """
// one
// two
fn f() {}
"""),
("a runnable doctest is exempt", """
/// one
/// ```
/// let x = 1;
/// assert_eq!(x, 1);
/// ```
/// two
fn f() {}
"""),
("a closed fence does not leak into a later comment", """
/// one
/// ```
/// let x = 1;
/// ```
fn a() {}
// two
// three
fn b() {}
"""),
]
def self_test():
"""Check the checker against every bypass it has already been fixed for.
Ten silent successes have been found in this file, every one failing in the
direction that looks like passing. See context/code/tooling.md.
"""
failures = []
for name, expected, src in SELF_TEST_CASES:
runs = [n for n in comment_runs(src.split("\n")) if n[1] > LIMIT]
longest = max((n[1] for n in runs), default=0)
if longest != expected:
failures.append(f" MISSED: {name}: expected a run of {expected}, saw {longest}")
for name, src in SELF_TEST_CLEAN:
runs = [n for n in comment_runs(src.split("\n")) if n[1] > LIMIT]
if runs:
failures.append(f" FALSE POSITIVE: {name}: flagged {runs}")
if failures:
print("checker self-test FAILED:", file=sys.stderr)
print("\n".join(failures), file=sys.stderr)
return 1
print(f"checker self-test: {len(SELF_TEST_CASES)} bypasses caught, "
f"{len(SELF_TEST_CLEAN)} legitimate shapes allowed")
return 0
def main():
ap = argparse.ArgumentParser()
ap.add_argument("roots", nargs="*",
@@ -187,7 +332,11 @@ def main():
help="list doc comments containing a ``` fence and exit")
ap.add_argument("--check-evidence", action="store_true",
help="every context/test-evidence heading must name a test that exists")
ap.add_argument("--self-test", action="store_true",
help="check the checker against the bypasses it has been fixed for")
args = ap.parse_args()
if args.self_test:
return self_test()
default_roots = ["src", "crates", "tests", "examples", "harness"]
requested = args.roots or default_roots
# A path that is not a directory must FAIL. Filtering it away made