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

check-ci-graph: a multi-line quoted value is refused, and the quote tracker is deleted (6zf1)

Browse Source 
The tracker closed a multi-line quote on an escaped \" or a doubled '', so a
later quote reopened it over a job's real if:. Both styles exited 0 on the real
ci.yml. The opening line is refused instead; _opens_quote's escape handling
is now load-bearing and has a case per style. Argument: context/code/tooling.md.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-23 18:20:36 -07:00
parent 588094b7bb
commit 9dc8a643b2
3 changed files with 154 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/tooling.md
+33 -10
View File
@@ -295,10 +295,10 @@ The gate reads no shell. It counts a step only when all of these hold:
away skips the call as surely as one on its own job (improvise-bbwa).
The only YAML it models is what makes a line structure rather than content. A
key with an inline value owns every following line indented deeper, and an
open quote owns every line until it closes. So a heredoc, a folded or plain
continuation, a quoted value or a whole fake job inside a top-level block
scalar is skipped as content. Any line outside a YAML comment that mentions
key with an inline value owns every following line indented deeper. So a
heredoc, a folded or plain continuation, or a whole fake job inside a
top-level block scalar is skipped as content. A quoted value must close on its
own line, or the line is refused (improvise-6zf1, below). Any line outside a YAML comment that mentions
`scripts/checkers.sh` and is not a counted step is **reported**, not ignored.
The union of the counted jobs' rows must cover `checkers.sh --list`.
@@ -358,12 +358,35 @@ There were two ways to make the matchers agree:
string, so no two matchers can read different versions of it. So the
stripper is deleted, not kept consistent.
It costs one valid style: a trailing comment, or a ` #` inside a quoted value,
on any line that is not block-scalar or quoted content. The real `ci.yml` has
neither. A full-line comment is still skipped, and a `#` with no space before
it, as in `nixpkgs#trealla`, is not a comment. After a refused line only the
exit is guaranteed. What the gate reads next may be garbled: a refused
`"x" # note` still opens the quote tracker. The exit is 1 either way.
This refuses some valid styles, on any line that is not block-scalar content:
- a trailing comment on a step, such as `- uses: actions/checkout@v7 # pin`;
- a one-line `run: echo "a # b"`;
- a quoted `with:` value containing ` #`.
This costs future readability, not correctness. The real `ci.yml` has none of
them. A full-line comment is still skipped, and a `#` with no space before it,
as in `nixpkgs#trealla`, is not a comment. After a refused line only the exit
is guaranteed: what the gate reads next may be garbled, and the exit is 1
either way.
**A quoted value that does not close on its own line is refused, and the quote
tracker is deleted** (improvise-6zf1). The tracker held a multi-line quoted
value open until the next line containing its quote character. YAML does not
close on an escaped `\"` inside double quotes, or on a doubled `''` inside
single quotes. So the tracker closed early, a later content line such as
`b: "` opened a phantom string, and that string swallowed the job's real
`if:`. Both quote styles exited 0 on the real `ci.yml`. This was the fourth
review round whose fail-open lived in quote or comment handling. Closing the
tracker correctly would have meant modelling YAML's quote rules one more time,
so the gate stops reading multi-line quotes at all. The real `ci.yml` has
none.
The opening line is still judged by `_opens_quote`, and that check is now
load-bearing: a value it wrongly calls closed is not refused, and its
continuation lines are read as structure. A `name: "a\"` followed by
`needs: ok` would then overwrite a job's real `needs: gate`. So `_opens_quote`
drops `\\.` before looking for the closing `"`, and `''` before looking for
the closing `'`, and a case covers each.
`read_graph` refuses nothing, so it cannot rely on refusal. It opens a job on
any ` name:` line followed by whitespace or the end of the line, which is
context/test-evidence/tooling.md
+66
View File
@@ -1177,3 +1177,69 @@ three. Four rivals are new:
The header suite case does not separate the raw-without-refusal rival. There
the exact `_JOB` on the raw line refuses the header anyway, and the complaint
has the same text.
### A multi-line quoted value is refused, and the quote tracker is gone — `scripts/check-ci-graph.py` (improvise-6zf1)
**What broke (P2, fail-open, predates j49r).** Found in the delta review of #228
at `588094b7`. The tracker closed a multi-line quoted value at the first line
containing its quote character, including an escaped `\"` or a doubled `''`,
which YAML does not treat as a close. A later content line such as `b: "` then
opened a phantom string that swallowed the job's real `if: false`. The review
planted a `gate` job whose `name:` spans lines, with `kb` given `needs: gate`
and `gate` added to pages-deploy's `needs`. yq-go read `gate.if == false`, and
the gate exited 0 in both quote styles.
**Fix.** A line whose quoted value does not close on its own line is refused,
with its own message. The tracker is deleted. The real `ci.yml` has no
multi-line quoted value (scanned with `_ANY_KEY` and `_opens_quote`: 0).
**Red first** against `588094b7`: both plant cases (double and single) gave
`saw []`.
**New cases, and why each is there.**
- The two plants. Each now draws the refusal, and also "conditional (needs
gate, which has if)", because with the tracker gone the job's `if:` is read.
- An opening line ending in an escaped `\"`, and one ending in a doubled `''`,
each followed by `needs: ok` and a readable closing line (`x: y"`). yq-go
reads each as one string, with `kb.needs == "gate"`. If `_opens_quote`
called either value closed, nothing would be refused, and the gate would
read `needs: ok` in place of kb's real need. These cases are what make
`_opens_quote`'s escape handling load-bearing and tested.
- Two refused styles the review listed: a trailing comment on a step, and
`run: echo "a # b"`. Both were already refused by the j49r rule, so these
cases document behaviour rather than fix a bug. The third style, a quoted
`with:` value containing ` #`, was already the j49r indent-10 case.
**Changed cases.**
- "an exact run: line inside a multi-line quoted value does not count" is now
"a run: line inside a multi-line quoted value: the quote is refused". It
draws the refusal of the opening line and a plain refusal of the closing
`b"`. Only the exit is guaranteed after a refused line.
- "a closed quote followed by a comment is refused (j49r)" now reads the
job-level `if:` correctly. Its expectation is the refusal plus "conditional
(job if)", which is what the j49r evidence wanted but could not have while
the tracker existed.
**Replayed on the real `ci.yml`.** I planted the review's three shapes: double
quote, single quote, and double quote with the reopening line at indent 4. At
`588094b7` each exited 0. Now each exits 1, naming the opening line. All seven
plants from both rounds were restored byte for byte (sha1 `a00288a8…`).
**Rivals, each a source edit run against the real `--self-test` (53 cases).**
All 18 fail:
- the review's eleven;
- the four from j49r;
- three new ones:
- a multi-line quote is not refused: fails the five 6zf1 cases;
- `_opens_quote` ignores `\"`: fails the escaped-`\"` case alone;
- `_opens_quote` ignores `''`: fails the doubled-`''` case alone.
The j49r rival "no comment refusal" now fails four cases, not three: the
`"x" # note` case is also caught by the quote refusal, because the raw value
`"x" # note` does not end in its quote.
**Checked, not a finding.** A flow value (`{…}` or `[…]`) that continues onto
another line is not refused. yq-go rejects a continuation at a lesser
indentation (`did not find expected ',' or '}'`), which is loud. At the key's
own indentation the continuation is read as a key, which fails closed: a
spurious `if:`, or a `needs:` value the exact `_NEEDS_VALUE` refuses.
scripts/check-ci-graph.py
+55 -28
View File
@@ -129,7 +129,7 @@ def ci_steps(text):
job, in_steps, job_flags, step = None, False, set(), None
in_jobs, job_keyed = False, False
workflow_flags = set()
owner, owner_step, quote = None, None, None
owner, owner_step = None, None
needs, if_jobs, in_needs = {}, set(), False
def close_step():
@@ -138,12 +138,6 @@ def ci_steps(text):
for n, raw in enumerate(text.split("\n"), 1):
indent = len(raw) - len(raw.lstrip())
if quote is not None:
if quote in raw:
quote = None
if "scripts/checkers.sh" in raw:
stray.append(n)
continue
if owner is not None and (not raw.strip() or indent > owner):
if "scripts/checkers.sh" in raw:
stray.append(n)
@@ -159,8 +153,11 @@ def ci_steps(text):
else:
readable = not in_jobs or indent > 8 or _readable(raw, indent, in_steps,
in_needs, job_keyed)
if _COMMENT.search(raw) or not readable:
unread.append((n, bool(_COMMENT.search(raw))))
k = _ANY_KEY.match(raw)
opens = bool(k) and _opens_quote(_inline_value(k.group(3)))
if _COMMENT.search(raw) or opens or not readable:
unread.append((n, "comment" if _COMMENT.search(raw) else
"quote" if opens else "plain"))
if raw.startswith("defaults:"):
workflow_flags.add("workflow defaults")
m = _JOB.match(raw)
@@ -207,14 +204,9 @@ def ci_steps(text):
stray.append(n)
elif "scripts/checkers.sh" in raw:
stray.append(n)
k = _ANY_KEY.match(raw)
if k and _inline_value(k.group(3)):
value = _inline_value(k.group(3))
if _opens_quote(value):
quote = value[0]
else:
owner = len(k.group(1)) + (2 if raw.lstrip().startswith("- ") else 0)
owner_step = step if m and m.group(1) == "run" else None
owner = len(k.group(1)) + (2 if raw.lstrip().startswith("- ") else 0)
owner_step = step if m and m.group(1) == "run" else None
close_step()
for s in steps:
s["flags"] |= workflow_flags
@@ -252,8 +244,11 @@ def _unrun(s):
"ci_suites in scripts/checkers.sh")
def _unread(n, comment=False):
if comment:
def _unread(n, kind="plain"):
if kind == "quote":
return (f"ci.yml line {n} opens a quoted value that continues onto another line; "
"this gate reads no multi-line quotes, so keep it on one line")
if kind == "comment":
return (f"ci.yml line {n} has a ` #`, which YAML may read as a comment; this gate "
"reads no comments, so put it on a line of its own")
return (f"ci.yml line {n} is a key or `needs:` value this gate cannot read as a "
@@ -321,11 +316,11 @@ _NEEDS_GATE = ["job `kb`'s `checkers.sh --ci` step is conditional (needs gate, w
"so it does not count"] + _NO_PW
def _unreadable(name, text, *markers, rest=(), comment=False):
def _unreadable(name, text, *markers, rest=(), kind="plain"):
"""A case whose workflow spells a key or a `needs:` value the gate must refuse."""
lines = text.split("\n")
at = [next(i for i, l in enumerate(lines, 1) if m in l) for m in markers]
return (name, _REQ, text, [_unread(n, comment) for n in at] + list(rest))
return (name, _REQ, text, [_unread(n, kind) for n in at] + list(rest))
# (name, suites to require, workflow text, the complaints it must draw). Every
@@ -337,7 +332,7 @@ SUITE_SELF_TEST = [
_unreadable("a trailing comment on the counted run: line is refused (j49r)",
_wf(_BUILD, _job("kb", _KB_RUN.rstrip("\n") + " # the kb row\n")),
"# the kb row", rest=[_stray(9)] + _NO_PW, comment=True),
"# the kb row", rest=[_stray(9)] + _NO_PW, kind="comment"),
("a job that never calls its row leaves its suites unrun", _REQ,
_wf(_BUILD, _job("kb", " - run: echo nothing\n")), _NO_PW),
@@ -355,10 +350,10 @@ SUITE_SELF_TEST = [
_wf(_BUILD, _job("kb", " - run: echo\n ./scripts/checkers.sh --ci kb\n")),
[_stray(10)] + _NO_PW),
("an exact run: line inside a multi-line quoted value does not count", _REQ,
_wf(_BUILD, _job("kb", " - name: \"a\n"
" run: ./scripts/checkers.sh --ci kb\n b\"\n")),
[_stray(10)] + _NO_PW),
_unreadable("a run: line inside a multi-line quoted value: the quote is refused (6zf1)",
_wf(_BUILD, _job("kb", " - name: \"a\n"
" run: ./scripts/checkers.sh --ci kb\n b\"\n")),
'- name: "a', kind="quote", rest=[_unread(11)]),
("a whole job inside a top-level block scalar does not count", _REQ,
_wf(_BUILD) + "notes: |\n kb:\n steps:\n"
@@ -543,21 +538,53 @@ SUITE_SELF_TEST = [
_wf(_BUILD, " test:\n runs-on: x\n",
" gate: # skipped on purpose\n if: false\n steps:\n - run: echo\n",
_job("kb", _KB_RUN, head=" needs: gate\n")),
"gate: # skipped", comment=True),
"gate: # skipped", kind="comment"),
_unreadable("a closed quote followed by a comment is refused (j49r)",
_wf(_BUILD, _job("kb", _KB_RUN, head=" env:\n A: \"x\" # note\n")
+ " if: false\n", " after:\n steps:\n - run: echo \"z\"\n"),
'"x" # note', rest=[_stray(11)] + _NO_PW, comment=True),
'"x" # note', kind="comment",
rest=["job `kb`'s `checkers.sh --ci` step is conditional (job if), "
"so it does not count"] + _NO_PW),
_unreadable("a # inside a quoted value, below indent 8, is refused (j49r)",
_wf(_BUILD, _job("kb", _KB_RUN + " - uses: x\n with:\n"
" foo: \"x # y\"\n")
+ " if: false\n", " after:\n steps:\n - run: echo \"z\"\n"),
'"x # y"', comment=True,
'"x # y"', kind="comment",
rest=["job `kb`'s `checkers.sh --ci` step is conditional (job if), "
"so it does not count"] + _NO_PW),
_unreadable("a trailing comment on a step is refused (j49r)",
_wf(_BUILD, _job("kb", " - uses: actions/checkout@v7 # pin\n" + _KB_RUN)),
"# pin", kind="comment"),
_unreadable("a one-line run: with a ` #` inside quotes is refused (j49r)",
_wf(_BUILD, _job("kb", " - run: echo \"a # b\"\n" + _KB_RUN)),
'"a # b"', kind="comment"),
_unreadable("a multi-line double-quoted value is refused, escaped quote and all (6zf1)",
_wf(_BUILD, " gate:\n runs-on: x\n name: \"a\n it\\\" is\n b: \"\n"
" if: false\n steps:\n - run: echo \"z\"\n",
_job("kb", _KB_RUN, head=" needs: gate\n")),
'name: "a', kind="quote", rest=_NEEDS_GATE),
_unreadable("a multi-line single-quoted value is refused, doubled quote and all (6zf1)",
_wf(_BUILD, " gate:\n runs-on: x\n name: 'a\n it''s\n b: '\n"
" if: false\n steps:\n - run: echo 'z'\n",
_job("kb", _KB_RUN, head=" needs: gate\n")),
"name: 'a", kind="quote", rest=_NEEDS_GATE),
_unreadable("an opening line ending in an escaped \\\" still opens a quote (6zf1)",
_wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: gate\n name: \"a\\\"\n"
" needs: ok\n x: y\"\n")),
'name: "a', kind="quote"),
_unreadable("an opening line ending in a doubled '' still opens a quote (6zf1)",
_wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: gate\n name: 'it''\n"
" needs: ok\n x: y'\n")),
"name: 'it", kind="quote"),
("a YAML comment mentioning checkers.sh is fine", _REQ,
_wf(_BUILD, _job("kb", " # every suite goes through scripts/checkers.sh\n"
" - run: ./scripts/checkers.sh --ci kb\n")), []),