From e37662e488b5f9346657b644d7ac43f39d2d0a3d Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Mon, 10 Aug 2026 14:20:41 -0700 Subject: [PATCH 1/4] test(ci): guard the run-block expression budget --- tests/workflow-lint-test.sh | 92 +++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/workflow-lint-test.sh b/tests/workflow-lint-test.sh index f377be8..08d751f 100755 --- a/tests/workflow-lint-test.sh +++ b/tests/workflow-lint-test.sh @@ -246,6 +246,98 @@ else echo "ok the smoke job grants exactly what the review job declares" fi +# Actions evaluates a `run:` block as a template, and the value it evaluates cannot exceed +# 21,000 characters. Going over does not fail the step -- it fails the whole workflow at load +# time, which is the same outage shape as the empty expression above: no jobs, the required +# check never reports, every open pull request in the org blocks. +# +# This has already happened once. #26 pushed the context step to 22,016 characters and had to +# be reverted (8b04393), which took an unrelated tool-usage flag down with it; #29 then moved +# that step out to the context script. Nothing in the suite could see either the breach or how +# close the file sat beforehand -- 20,545 characters, about eight comment lines of headroom. +# +# So the budget is 20,000, not 21,000: a block within a few comments of the ceiling is the +# defect, because the next person adding a comment is the one who takes the org down. Blocks +# over budget belong in scripts/ like the context step, not trimmed to fit. +# +# The measurement has to match what Actions counts, which is the block scalar's *value* -- +# indentation stripped. Counting raw lines overstates every block (the context step reads +# 24,285 that way) and would make the budget meaningless. Implemented against the standard +# library only: this is the check for the defect that caused an outage, so it cannot be the +# one that skips when a YAML module is missing. +RUN_BUDGET=20000 +oversized=$(python3 - "$RUN_BUDGET" "${WORKFLOWS[@]}" <<'PY' +import sys + +budget, paths = int(sys.argv[1]), sys.argv[2:] +found = 0 +for path in paths: + with open(path) as fh: + lines = fh.readlines() + i = 0 + while i < len(lines): + stripped = lines[i].rstrip("\n") + key = stripped.lstrip() + if key.startswith("run:"): + rest = key[4:].strip() + key_indent = len(stripped) - len(key) + start = i + 1 + if not rest.startswith(("|", ">")): + # Single-line plain scalar: the value is the text, no trailing newline. + found += 1 + size = len(rest) + i += 1 + else: + i += 1 + body, block_indent = [], None + while i < len(lines): + raw = lines[i].rstrip("\n") + if not raw.strip(): # blank lines belong to the block + body.append("") + i += 1 + continue + indent = len(raw) - len(raw.lstrip()) + if indent <= key_indent: + break + if block_indent is None: + block_indent = indent + body.append(raw[block_indent:]) + i += 1 + # Clip chomping: trailing blank lines collapse to the single closing newline. + while body and body[-1] == "": + body.pop() + found += 1 + # Exact for the `|` family, which is what every block over a few hundred + # characters here uses. A folded `>` joins lines with spaces, so this + # over-counts it by the newlines -- erring toward failing early, which is the + # safe direction for a budget. + size = len("\n".join(body) + "\n") if body else 0 + if size >= budget: + print(f"{path}:{start}: run block is {size} characters, " + f"budget {budget} (Actions rejects the workflow at 21000)") + continue + i += 1 +if not found: + print("NO-RUN-BLOCKS-FOUND") +PY +) || { + echo "FAIL the run-block size scan itself failed; the budget check proves nothing" + failures=$((failures + 1)) + oversized="" +} +if [ "$oversized" = "NO-RUN-BLOCKS-FOUND" ]; then + echo "FAIL the run-block scan found no run: blocks at all; the budget check proves nothing" + failures=$((failures + 1)) +elif [ -n "$oversized" ]; then + echo "FAIL a run: block is at or over the ${RUN_BUDGET}-character budget. Actions fails the" + echo " whole workflow at 21000 -- no jobs, no required check, every org PR blocked." + echo " Move the script to scripts/ rather than trimming comments to fit:" + printf '%s\n' "$oversized" | sed 's/^/ /' + failures=$((failures + 1)) +else + echo "ok every run: block is under the ${RUN_BUDGET}-character budget" +fi + # The scan above is a backstop for one class. actionlint checks the schema, the expression # grammar, and the shell; run it when it is on PATH. shellcheck findings are excluded because # the run blocks here intentionally use unquoted word splitting for job ids. From ff2094d554e7ab65a654de0a7f05037a4a21d040 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Mon, 10 Aug 2026 14:20:49 -0700 Subject: [PATCH 2/4] feat(review): flag command substitution in the tool usage artifact --- .github/workflows/claude-pr-review.yml | 27 ++++++++- README.md | 19 ++++-- tests/fixtures/execution-log-review-body.json | 50 ++++++++++++++++ tests/tool-usage-test.sh | 60 ++++++++++++++++++- 4 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 tests/fixtures/execution-log-review-body.json diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml index e6022b5..933b434 100644 --- a/.github/workflows/claude-pr-review.yml +++ b/.github/workflows/claude-pr-review.yml @@ -277,7 +277,30 @@ jobs: # norm strips the wrappers the reviewer puts in front of a real command (timeout, # cd .. &&, env VAR=x) so they do not all collapse into "other". verb returns the # first matching label or "other" -- the output is always one of these literals. - CMD_JQ='def norm: sub("^\\s+"; "") | sub("^timeout\\s+[0-9]+m?\\s+"; "") | sub("^cd\\s+[^&|;]+&&\\s*"; "") | sub("^env\\s+\\S+=\\S+\\s+"; ""); def verb: . as $c | ([[["^gh\\s+pr\\s+diff", "gh pr diff"], ["^gh\\s+pr\\s+view", "gh pr view"], ["^gh\\s+pr\\s+checks", "gh pr checks"], ["^gh\\s+pr\\s+review", "gh pr review"], ["^gh\\s+pr\\s+comment", "gh pr comment"], ["^gh\\s+api", "gh api"], ["^gh\\s", "gh other"], ["^git\\s+diff", "git diff"], ["^git\\s+log", "git log"], ["^git\\s+show", "git show"], ["^git\\s+blame", "git blame"], ["^git\\s", "git other"], ["^rg\\b", "rg"], ["^grep\\b", "grep"], ["^(fd|find)\\b", "find"], ["^(ls|tree)\\b", "ls"], ["^(sed|awk)\\b", "sed/awk"], ["^(cat|head|tail|wc)\\b", "cat/head/tail"], ["^(pytest|uv|python3?|cargo|npm|pnpm|yarn|bun|node|go|make|ruff|mypy|pyflakes)\\b", "run tests/build"]][] | select(.[0] as $re | $c | test($re))] | .[0] // ["", "other"]) | .[1]; def unquoted: gsub("\"[^\"]*\""; "") | gsub("\u0027[^\u0027]*\u0027"; ""); def classify: {cmd: (norm | verb), compound: (unquoted | test("\\||&&|;|>"))}; def toolname: if type == "string" and test("^[A-Za-z0-9_-]{1,64}$") then . else "unknown" end;' + CMD_JQ='def norm: sub("^\\s+"; "") | sub("^timeout\\s+[0-9]+m?\\s+"; "") | sub("^cd\\s+[^&|;]+&&\\s*"; "") | sub("^env\\s+\\S+=\\S+\\s+"; ""); def verb: . as $c | ([[["^gh\\s+pr\\s+diff", "gh pr diff"], ["^gh\\s+pr\\s+view", "gh pr view"], ["^gh\\s+pr\\s+checks", "gh pr checks"], ["^gh\\s+pr\\s+review", "gh pr review"], ["^gh\\s+pr\\s+comment", "gh pr comment"], ["^gh\\s+api", "gh api"], ["^gh\\s", "gh other"], ["^git\\s+diff", "git diff"], ["^git\\s+log", "git log"], ["^git\\s+show", "git show"], ["^git\\s+blame", "git blame"], ["^git\\s", "git other"], ["^rg\\b", "rg"], ["^grep\\b", "grep"], ["^(fd|find)\\b", "find"], ["^(ls|tree)\\b", "ls"], ["^(sed|awk)\\b", "sed/awk"], ["^(cat|head|tail|wc)\\b", "cat/head/tail"], ["^(pytest|uv|python3?|cargo|npm|pnpm|yarn|bun|node|go|make|ruff|mypy|pyflakes)\\b", "run tests/build"]][] | select(.[0] as $re | $c | test($re))] | .[0] // ["", "other"]) | .[1]; def unquoted: gsub("\"[^\"]*\""; "") | gsub("\u0027[^\u0027]*\u0027"; ""); def classify: {cmd: (norm | verb), compound: (unquoted | test("\\||&&|;|>")), has_subst: test("`|\\$\\(")}; def toolname: if type == "string" and test("^[A-Za-z0-9_-]{1,64}$") then . else "unknown" end;' + # + # has_subst is the same kind of flag for the denials that outlived the frontloaded + # context. Reads mostly stopped being refused once the context arrived in the prompt + # -- denials fell from 5.2 a run to 0.5 -- and what is left is the *write* path: + # `gh pr review` is allowlisted and still refused on 29% of its 241 attempts across + # 170 settled-window runs and 8 repos, 1.6 times per affected run before the review + # lands, costing those runs +$0.46 and +73s each (issue #33). compound reported 1 of + # those 71, which is the point: it cannot see this. The standing hypothesis is the + # review body rather than the command -- a body is markdown, and a backtick inside a + # double-quoted argument is command substitution to anything parsing shell. So the + # flag is tested against the *raw* command, not the unquoted form compound uses: + # stripping quoted spans first would remove precisely the backticks in question. It + # rides on `commands` as well as `denied_commands` because a denial rate needs its + # base rate to mean anything. Boolean, like compound: a label, never a span of the + # command. + # + # This shipped once before, in #26, and was reverted in 8b04393 -- not on its own + # merits: #26 pushed the *context* step's run block to 22,016 characters, past the + # 21,000-character expression limit, and the revert took this with it. #29 then moved + # that step out to scripts/gather-review-context.sh, so the budget that forced the + # revert is gone. The step below is 3.3k with the flag; the guard in + # tests/workflow-lint-test.sh now fails before a run block can reach the limit again. + # # commands and denied_commands answer two different questions: what the reviewer # spends its Bash budget on, and which of those the allowlist refuses. compound is # carried separately because an allowlisted command still gets denied when it is @@ -285,7 +308,7 @@ jobs: # tested against the command with quoted spans removed, because `rg -n \"a|b\"` is # one allowlisted command and counting its alternation as a pipe would inflate # exactly the number the flag exists to produce. - TOOL_USAGE_JQ='{tool_calls: ([.[]? | select(.type=="assistant") | .message.content[]? | select(.type=="tool_use") | .name | toolname] | group_by(.) | map({name: .[0], n: length}) | sort_by(-.n)), commands: ([.[]? | select(.type=="assistant") | .message.content[]? | select(.type=="tool_use" and .name=="Bash") | (.input.command // "") | classify] | group_by([.cmd, .compound]) | map({cmd: .[0].cmd, compound: .[0].compound, n: length}) | sort_by(-.n)), denials: (([.[]? | select(.type=="result")] | last // {}) | (.permission_denials // []) | map(.tool_name | toolname) | group_by(.) | map({name: .[0], n: length}) | sort_by(-.n)), denied_commands: (([.[]? | select(.type=="result")] | last // {}) | (.permission_denials // []) | map(select(.tool_name == "Bash") | (.tool_input.command // "") | classify) | group_by([.cmd, .compound]) | map({cmd: .[0].cmd, compound: .[0].compound, n: length}) | sort_by(-.n)), result: (([.[]? | select(.type=="result")] | last // {}) | {subtype, is_error, num_turns, duration_ms, total_cost_usd})}' + TOOL_USAGE_JQ='{tool_calls: ([.[]? | select(.type=="assistant") | .message.content[]? | select(.type=="tool_use") | .name | toolname] | group_by(.) | map({name: .[0], n: length}) | sort_by(-.n)), commands: ([.[]? | select(.type=="assistant") | .message.content[]? | select(.type=="tool_use" and .name=="Bash") | (.input.command // "") | classify] | group_by([.cmd, .compound, .has_subst]) | map({cmd: .[0].cmd, compound: .[0].compound, has_subst: .[0].has_subst, n: length}) | sort_by(-.n)), denials: (([.[]? | select(.type=="result")] | last // {}) | (.permission_denials // []) | map(.tool_name | toolname) | group_by(.) | map({name: .[0], n: length}) | sort_by(-.n)), denied_commands: (([.[]? | select(.type=="result")] | last // {}) | (.permission_denials // []) | map(select(.tool_name == "Bash") | (.tool_input.command // "") | classify) | group_by([.cmd, .compound, .has_subst]) | map({cmd: .[0].cmd, compound: .[0].compound, has_subst: .[0].has_subst, n: length}) | sort_by(-.n)), result: (([.[]? | select(.type=="result")] | last // {}) | {subtype, is_error, num_turns, duration_ms, total_cost_usd})}' jq "$CMD_JQ $TOOL_USAGE_JQ" "$EXECUTION_FILE" > "${RUNNER_TEMP}/claude-tool-usage.json" env: # Via env, not a ${{ }} interpolation inside the script, so the path cannot be diff --git a/README.md b/README.md index 2ad2c5a..9a22805 100644 --- a/README.md +++ b/README.md @@ -51,10 +51,21 @@ them would otherwise end the data block early and land the rest where it reads a ### Tool usage artifact Each run attaches a `claude-tool-usage-pr-` artifact (14-day retention): tool call counts, -Bash command labels with a compound flag, the denied subset of both, and the run's turn count and -cost. It exists to diagnose permission denials against the workflow's `--allowedTools` list, since -the job log records only the number of denials, never what was refused. Tool names alone proved -insufficient — 520 of 567 denials in the first week were `Bash`, which is every command there is. +Bash command labels with a compound flag and a command-substitution flag, the denied subset of both, +and the run's turn count and cost. It exists to diagnose permission denials against the workflow's +`--allowedTools` list, since the job log records only the number of denials, never what was refused. +Tool names alone proved insufficient — 520 of 567 denials in the first week were `Bash`, which is +every command there is. + +The two flags are deliberately measured differently, and the difference is the point. `compound` +strips quoted spans before looking for `| && ; >`, because `rg -n "a|b"` is one allowlisted command +and counting its alternation as a pipe would inflate the number the flag exists to produce. +`has_subst` tests the raw command for `` ` `` and `$(`, because the suspected trigger lives *inside* +the quoted body: a review body is markdown, and a backtick in a double-quoted argument is command +substitution to anything parsing shell. Frontloading the context fixed the read path — denials fell +from 5.2 a run to 0.5 — but `gh pr review` is allowlisted and still refused on 29% of its 241 +attempts across 170 runs and 8 repos, at +$0.46 and +73s per affected run, with `compound` reporting +1 of those 71. Measuring `has_subst` the same way as `compound` would have kept that invisible. The artifact is a projection of the action's execution log, never the log itself — that file is the full conversation, and the runner holds a git credential the reviewer can read, which artifacts diff --git a/tests/fixtures/execution-log-review-body.json b/tests/fixtures/execution-log-review-body.json new file mode 100644 index 0000000..561afc3 --- /dev/null +++ b/tests/fixtures/execution-log-review-body.json @@ -0,0 +1,50 @@ +[ + { + "type": "assistant", + "message": { + "role": "assistant", + "content": [ + { + "type": "tool_use", + "id": "t1", + "name": "Bash", + "input": { + "command": "gh pr review 308 --request-changes --body \"**Blocking:** `components/GetUpdates.tsx:5` hardcodes the endpoint. Route it through `app/api/get-updates/route.ts` and apply `rateLimit(ip, 5, 60000 * 10)`.\"" + } + } + ] + } + }, + { + "type": "assistant", + "message": { + "role": "assistant", + "content": [ + { + "type": "tool_use", + "id": "t2", + "name": "Bash", + "input": { + "command": "gh pr review 308 --request-changes --body \"Blocking: components/GetUpdates.tsx line 5 hardcodes the endpoint. Route it through app/api/get-updates/route.ts and apply the rate limiter.\"" + } + } + ] + } + }, + { + "type": "result", + "subtype": "success", + "is_error": false, + "num_turns": 11, + "duration_ms": 151402, + "total_cost_usd": 0.8021, + "permission_denials": [ + { + "tool_name": "Bash", + "tool_input": { + "command": "gh pr review 308 --request-changes --body \"**Blocking:** `components/GetUpdates.tsx:5` hardcodes the endpoint. Route it through `app/api/get-updates/route.ts` and apply `rateLimit(ip, 5, 60000 * 10)`.\"" + } + } + ] + } +] diff --git a/tests/tool-usage-test.sh b/tests/tool-usage-test.sh index 6cce860..6606d53 100755 --- a/tests/tool-usage-test.sh +++ b/tests/tool-usage-test.sh @@ -141,6 +141,64 @@ expect_compound "rg -n 'a|b' src/ | head -20" true \ expect_compound 'rg -n foo src/' false \ "a plain search is not compound" +# has_subst answers the denial the frontloaded context did not remove: an allowlisted +# `gh pr review` refused on the way to posting. Unlike compound it is tested against the raw +# command, because the suspected trigger lives *inside* the quoted body -- a review body is +# markdown, and backticks in a double-quoted argument are command substitution to anything +# parsing shell. Running it through `unquoted` first would delete the evidence. +# +# Measured before this landed: `gh pr review` is refused on 29% of its 241 attempts across +# 170 settled-window runs, on 8 repos, costing the affected runs +$0.46 and +73s each -- +# while `compound` reported 1 of 71. See issue #33. +subst_of() { + printf '%s' "$1" | jq -R -r "$CMD_JQ classify | .has_subst | tostring" +} +# expect_subst +expect_subst() { + local actual + actual=$(subst_of "$1") + if [ "$actual" = "$2" ]; then + echo "ok $3" + else + echo "FAIL $3: expected has_subst=$2, got $actual for: $1" + failures=$((failures + 1)) + fi +} + +expect_subst 'gh pr review 21 --approve --body "nit: `foo` is wrong"' true \ + "a backtick inside the review body is flagged" +expect_subst 'gh pr comment 21 --body "see $(basename x)"' true \ + "an explicit command substitution is flagged" +expect_subst 'gh pr review 21 --approve --body "no markdown here"' false \ + "a plain body is not flagged" +expect_subst 'rg -n foo src/' false \ + "a plain search is not flagged" +# The distinction from compound, stated as an assertion: quoted spans are removed for one +# flag and kept for the other, so a body whose only shell-ish characters are backticks is +# has_subst without being compound. Getting these the same way round would make the two +# columns redundant and lose the write-path denials again. +expect_compound 'gh pr review 21 --approve --body "nit: `foo` is wrong"' false \ + "a backtick in a quoted body is not compound" + +# End to end over the shape actually seen in production: the reviewer's first +# `gh pr review --request-changes` was refused, and the retry that landed carried the same +# feedback with the backticks removed. Both rows are `gh pr review`; has_subst is the only +# thing that tells them apart, which is the whole reason it is grouped on. +expect_jq execution-log-review-body.json \ + '[.commands[] | {cmd, has_subst, n}] | sort_by(.has_subst)' \ + '[{"cmd":"gh pr review","has_subst":false,"n":1},{"cmd":"gh pr review","has_subst":true,"n":1}]' \ + "the flagged and unflagged attempts are counted apart" +expect_jq execution-log-review-body.json '.denied_commands' \ + '[{"cmd":"gh pr review","compound":false,"has_subst":true,"n":1}]' \ + "the denied review post is flagged and not compound" + +# Same boundary as every other label: the flag is a boolean, so no part of the body it was +# computed from may ride along with it. +expect_absent execution-log-review-body.json "GetUpdates.tsx" \ + "the review body does not reach the artifact" +expect_absent execution-log-review-body.json "rateLimit" \ + "code quoted in the review body does not reach the artifact" + # The containment assertion, and the one that has to keep holding: every label the # projection emits is a literal in CMD_JQ. Nothing derived from the transcript can satisfy # it, so the artifact cannot grow a credential path, a search pattern, or a file name @@ -202,7 +260,7 @@ if printf '%s' "$leaked" | grep -qF "ghs_FAKETOKENFORTESTS" \ || printf '%s' "$leaked" | grep -qF "curl"; then echo "FAIL unrecognised command leaked into the projection: $leaked" failures=$((failures + 1)) -elif printf '%s' "$leaked" | jq -e '.commands == [{"cmd":"other","compound":false,"n":1}]' >/dev/null; then +elif printf '%s' "$leaked" | jq -e '.commands == [{"cmd":"other","compound":false,"has_subst":false,"n":1}]' >/dev/null; then echo "ok unrecognised command reduces to \"other\"" else echo "FAIL unrecognised command did not reduce to \"other\": $leaked" From 522adae94ca9946a25ca05137184481c98d49e81 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Mon, 10 Aug 2026 14:28:38 -0700 Subject: [PATCH 3/4] fix(ci): measure run blocks written as a step first key --- tests/workflow-lint-test.sh | 45 +++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/tests/workflow-lint-test.sh b/tests/workflow-lint-test.sh index 08d751f..0d88e75 100755 --- a/tests/workflow-lint-test.sh +++ b/tests/workflow-lint-test.sh @@ -266,7 +266,9 @@ fi # library only: this is the check for the defect that caused an outage, so it cannot be the # one that skips when a YAML module is missing. RUN_BUDGET=20000 -oversized=$(python3 - "$RUN_BUDGET" "${WORKFLOWS[@]}" <<'PY' +SCANNER=$(mktemp) +trap 'rm -f "$SCANNER" "$SCANNER.yml"' EXIT +cat > "$SCANNER" <<'PY' import sys budget, paths = int(sys.argv[1]), sys.argv[2:] @@ -278,8 +280,17 @@ for path in paths: while i < len(lines): stripped = lines[i].rstrip("\n") key = stripped.lstrip() + # `- run: |` is a run block too -- a step may put run first, with no name. Skipping it + # would leave the largest kind of block unmeasured while the check still reported ok, + # which is the failure this file exists to prevent. The list marker is part of the + # key's indentation: ` - run:` puts the step map at column 8, so content has to + # be indented past 8, not past 6. + while key.startswith("- "): + key = key[2:] if key.startswith("run:"): rest = key[4:].strip() + # Measured off the stripped key, so the list marker counts as indentation: + # ` - run:` gives 8, the column `run` actually sits at. key_indent = len(stripped) - len(key) start = i + 1 if not rest.startswith(("|", ">")): @@ -320,7 +331,37 @@ for path in paths: if not found: print("NO-RUN-BLOCKS-FOUND") PY -) || { + +# The scanner is checked against known sizes before it is trusted on the real files. A +# sentinel that only proves blocks were *found* cannot distinguish a correct measurement from +# one that reads every block as zero -- and a budget check that always measures low reports ok +# forever. Both step shapes appear here, because the `- run:` form was missed at first review +# and would have gone unmeasured with the check still green. Sizes are countable by eye: two +# 9-character lines plus their newlines is 20, one 10-character line plus its newline is 11. +cat > "$SCANNER.yml" <<'YML' +name: selftest +on: push +jobs: + a: + runs-on: ubuntu-latest + steps: + - run: | + aaaaaaaaa + bbbbbbbbb + - name: named step + run: | + cccccccccc +YML +measured=$(python3 "$SCANNER" 1 "$SCANNER.yml" | sed 's/.*run block is \([0-9]*\) characters.*/\1/' | sort -n | tr '\n' ' ') +if [ "$measured" != "11 20 " ]; then + echo "FAIL the run-block scanner mis-measures a known input: expected sizes '11 20 ', got" + echo " '$measured' -- so the budget check below cannot be trusted" + failures=$((failures + 1)) +else + echo "ok the run-block scanner measures both step shapes correctly" +fi + +oversized=$(python3 "$SCANNER" "$RUN_BUDGET" "${WORKFLOWS[@]}") || { echo "FAIL the run-block size scan itself failed; the budget check proves nothing" failures=$((failures + 1)) oversized="" From eff75a4a0bcf25288c6bcd402463c30a37d0a5c0 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Mon, 10 Aug 2026 14:33:02 -0700 Subject: [PATCH 4/4] test(ci): cover the plain-scalar run shape in the scanner self-test --- tests/workflow-lint-test.sh | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/workflow-lint-test.sh b/tests/workflow-lint-test.sh index 0d88e75..eee890e 100755 --- a/tests/workflow-lint-test.sh +++ b/tests/workflow-lint-test.sh @@ -335,9 +335,16 @@ PY # The scanner is checked against known sizes before it is trusted on the real files. A # sentinel that only proves blocks were *found* cannot distinguish a correct measurement from # one that reads every block as zero -- and a budget check that always measures low reports ok -# forever. Both step shapes appear here, because the `- run:` form was missed at first review -# and would have gone unmeasured with the check still green. Sizes are countable by eye: two -# 9-character lines plus their newlines is 20, one 10-character line plus its newline is 11. +# forever. +# +# All three shapes the scanner branches on appear here: `- run: |` with the run key first (this +# was missed at first review and would have gone unmeasured with the check still green), the +# same block under a `name:`, and a plain single-line `run:` -- which is the most common shape +# in these files and the one branch that has no block-scalar logic to fall back on. +# +# Sizes are countable by eye, so the assertion needs no YAML parser to justify: two 9-character +# lines plus their newlines is 20, one 10-character line plus its newline is 11, and a plain +# scalar is its text with no trailing newline, so `echo hello` is 10. cat > "$SCANNER.yml" <<'YML' name: selftest on: push @@ -351,14 +358,16 @@ jobs: - name: named step run: | cccccccccc + - name: plain scalar + run: echo hello YML measured=$(python3 "$SCANNER" 1 "$SCANNER.yml" | sed 's/.*run block is \([0-9]*\) characters.*/\1/' | sort -n | tr '\n' ' ') -if [ "$measured" != "11 20 " ]; then - echo "FAIL the run-block scanner mis-measures a known input: expected sizes '11 20 ', got" +if [ "$measured" != "10 11 20 " ]; then + echo "FAIL the run-block scanner mis-measures a known input: expected sizes '10 11 20 ', got" echo " '$measured' -- so the budget check below cannot be trusted" failures=$((failures + 1)) else - echo "ok the run-block scanner measures both step shapes correctly" + echo "ok the run-block scanner measures all three run: shapes correctly" fi oversized=$(python3 "$SCANNER" "$RUN_BUDGET" "${WORKFLOWS[@]}") || {