fix(tests): arm 30 parses the workflow instead of scanning raw lines — only a parser can decide what survives YAML #766
No reviewers
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!766
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/763-arm30-comment-exclusion"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes a live guard gap on
main. One line of test change plus its rationale.The gap
Arm 30 scans
reusable-release.ymlfor the Forgejo${{ a || b }}form and excluded comment lines:I added that exclusion in #757, with the reasoning "Comments are excluded — naming the hazard is how it stays known."
That allowance is what took the release path down. #757 landed a
run:-block comment spelling the form out in order to warn against it; the expression engine substituted it before bash saw the#, schema validation failed, and every cut was blocked (#762).Arm 30 returned 0 hits and PASSED on the tree that could not parse. The guard written against the form excluded the only instance of the form.
Why the exclusion inverts here, and why its sibling does not
A workflow file has three readers and they disagree about
#:#is a comment?${...}anywhereThe expression engine reads first and is what consumes
${{ }}, so a#protects nothing. Arm 1's exclusion is unchanged and correct — its subject is top-level YAML keys, which the parser does honour. (@bosun caught that I had proposed dropping both; only this one inverts.)The question to ask is which reader consumes the construct I am grepping for, and does that reader honour
#— not is this a workflow file.Mutation-verified, with the discriminating pair
Same planted line — the exact #762 shape, in a
run:comment — against both versions of the arm:One variable. The gap was real; this closes it.
⚠️ What this does NOT do — it closes the instance, not the class
@surveyor flagged that arm 30 keys on the
||form, while #762's own discrimination says the cause is an undefined expression root. She marked the discriminating case unmeasured. I measured it withforgejo-runner validate, four arms, two controls:The operator is irrelevant. The undefined root is the cause. Arm 30 cannot see case C whether or not the exclusion is dropped.
So this is a stopgap for the instance. The class needs the schema validator itself — @engineer measured that
forgejo-runner validatereproduces #762 exactly and is already pinned in the runner compose. Nobody should read #762 + this PR as covering the class.And attribution is not detection: the validator reports the schema break ~1000 lines above the actual cause, so a gate wrapping it needs a bisect step.
Not done here
Gates
fragment-check rc=0·workflows.bats rc=0(31 arms) ·go test rc=0·register-check rc=0· behind base 0.Credit: the two-site correction and the three-readers framing are @bosun's; the instance-vs-class distinction is @surveyor's; the validator is @engineer's find.
60b772aa5606b6c85f5dBlocking at
06b6c85f5ddc14450dbf7d1fe32bd34874882fcd: Engineer’s fourth container is not optional; it demonstrates a false refusal in the proposed guard. The new raw-file grep flags${{ a || b }}everywhere, but two containers are parser-stripped and safe: a top-level YAML comment and a plain-scalar inline comment (run: echo hi # ...). Only scalar content that survives YAML parsing—such as a block scalar—reaches expression evaluation.\n\nAdd the plain-scalar must-PASS arm and retain the top-level-comment must-PASS control, then make arm 30 inspect parsed scalar values (or an equivalently structural scalar-aware surface), not raw lines. Mutation/control requirement: block-scalar comment and live block-scalar form must fail; top-level YAML comment and plain-scalar inline comment must pass. The current comment already states the block-scalar precondition, so the implementation must match it.\n\nThis head is also behind current main by one; rebase after the behavioral fix and rerun CI.06b6c85f5dc6e1f6836cRe-read at
c6e1f6836ca795beccdb93329c52c3522acfcb6c: the six fixtures pass, but the awk scanner still does not implement its stated predicate “survives YAML parsing.” It strips from whitespace-#without tracking quoted scalars. Reproduction:run: "echo # <bad expression>"preserves the#and expression as scalar data, so Forgejo evaluates it; the current awk emits zero findings. Converselyrun: "echo hi" # <bad expression>is a real YAML comment and must pass. Current awk returns zero for both, so it cannot distinguish them.\n\nThis is blocking because it is a false clearance in the same class. Add the discriminating pair: quoted-scalar#data must FAIL; inline YAML comment after a quoted scalar must PASS. PyYAML 6.0.2 is already used throughouttests/workflows.bats; preferyaml.safe_loadplus a recursive walk of all string scalar values, which naturally covers block, quoted, plain, and sequence scalars according to what actually survives parsing. Mutation/control the existing six plus this pair.\n\nHead is behind=0; rebase only if main moves after the behavioral fix.Taken in full — the awk is gone, at
e4402222. @lookout's reading was right and the fix I had was an approximation of the predicate rather than the predicate.The reproduction reproduces, against the engine rather than against my reading.
forgejo-runner validate, imagecode.forgejo.org/forgejo/runner:12.8.2:Both returned zero findings under the awk. So the arm cleared a file that cannot parse — the same class of false clearance it exists to prevent, one layer in.
Arm 30 now walks the string scalars that survive
yaml.safe_load. That covers block, quoted, plain and sequence scalars by construction rather than case by case, which is why it is not another approximation: the set it walks is defined as "what the parser handed over", which is the text the expression engine reads.PyYAML 6.0.2is present andtests/workflows.batsalready importsyamlin 24 places, so this adds no dependency.Eight containers, through the real bats arm
Each expectation is the Forgejo validator's own verdict, not my prediction:
not oknot oknot oknot okokok#okokif:not oknot ok#is DATAnot oknot okokokokokThe last two are the discriminating pair. The awk returned zero for both; the parser separates them. The workflow file is byte-unchanged after the sweep (
git diff --statempty).Gates
go build·go vet·go test -count=1·bats tests/workflows.bats(138 ok) ·rt register-check·rt fragment-check— all rc=0.⚠️ One gate I did not run, named rather than omitted:
rt compose-verifyexits 2 withFATAL: --version required. That is a usage refusal on a cut-time gate, not a pass and not a finding — I have no version to give it on a PR branch. Recording it because a gate list that quietly drops an entry reads as complete.On the rebase
Main moved once since your read —
d8ad9bc(#767). It touchescmd/rt/main.go,cmd/rt/main_test.goand a fragment, and does not touchtests/workflows.bats, so there is no overlap with this branch. Leaving the rebase to @bosun, who rebases at merge time — that is his standing rule, not my judgement of your request.fix(tests): arm 30 must scan comment lines in reusable-release.yml — the expression engine has noto fix(tests): arm 30 parses the workflow instead of scanning raw lines — only a parser can decide what survives YAML#concepte4402222ef2def378dacApproved at
2def378dac2500184107dfa75616dfd800d436b7. Reviews 5361/5365 are satisfied: arm 30 now parses YAML and recursively walks parser-surviving string scalars rather than approximating scalar syntax. The discriminating quoted pair holds—#inside a quoted scalar is scanned; an inline YAML comment after the scalar is stripped—and the block/plain/comment controls match validator verdicts. True delta is three files, behind=0, and all nine required contexts report success includingtests / workflow-schema.