devantler-tech / devantler-tech/ksail
ci harness: default-branch matcher silently skips GitHub `+`/`?` filter patterns
- Dominant language
- Go
- Stars
- 165
- Forks
- 12
- Avg merge
- 5h 51m
- Merged PRs (30d)
- 347
Description
> 🤖 Generated by the Agentic Engineer
## Evidence
`matchesDefaultBranch` in `internal/ciharness/ci_workflow_test.go` (added by #6506, merged
`ea76b9ae38d9`) decides which workflows the cancel-in-progress guard applies to. Its stated
contract is in its own comment:
> A malformed pattern, or one using syntax GitHub accepts and Go does not, errors here.
> Treat that as a match: this function fails open toward inclusion.
The second half of that premise is false. GitHub's filter-pattern syntax gives `?` and `+`
quantifier meanings over the **preceding character** (`?` = zero or one, `+` = one or more),
whereas `filepath.Match` treats `?` as "any single character" and `+` as a literal. Neither
raises an error, so `err != nil` never fires and the pattern is silently `continue`d.
Reproduced directly (Go, `path/filepath`):
```
"main" matched=true err=
"ma?in" matched=false err= <-- GitHub: matches main
"mai+n" matched=false err= <-- GitHub: matches main
"m+ain" matched=false err= <-- GitHub: matches main
"mai?n" matched=false err= <-- GitHub: matches main
"maa?in" matched=false err= <-- GitHub: matches main
"[main" matched=false err=syntax error in pattern <-- the only case that errors
```
Only a genuine syntax error reaches the `whenUnparseable` branch. For the two GitHub
metacharacters the function fails **closed**, against its own documented direction.
## Impact
A workflow whose branch filter used `+` or `?` would be silently dropped from the guard's
checked set — the guard would report clean while not covering it. That is the precise hole
the comment says the matcher "exists to close".
**Live exposure is currently zero**: no workflow in `.github/workflows/` uses `+` or `?` in a
`branches:`/`branches-ignore:` filter today. This is a latent robustness gap plus an
incorrect code comment that will mislead the next reader, not a live miss.
## Expected behaviour
A pattern containing a metacharacter whose GitHub meaning `filepath.Match` does not
reproduce is treated as unparseable, so `whenUnparseable` decides it — the same
fail-open-toward-inclusion the comment already promises.
## Acceptance criteria
- [ ] `matchesDefaultBranch` routes patterns containing GitHub-specific quantifier syntax to
the `whenUnparseable` branch rather than silently skipping them.
- [ ] RED/GREEN proof: a test asserting `runsOnDefaultBranch` includes a workflow gated on
`mai+n` / `ma?in` fails before the change and passes after.
- [ ] The code comment states the actual `filepath.Match` behaviour, including that `+`/`?`
do not error.
Rough size: one-line matcher change plus table-test cases.
Contributor guide
Research direction
Start with matchesDefaultBranch in internal/ciharness/ci_workflow_test.go and the existing runsOnDefaultBranch tests. Run the relevant Go tests, then add table cases for mai+n and ma?in so they reach the whenUnparseable path, and update the comment to describe filepath.Match accurately. Done means the RED/GREEN cases pass and the documented fail-open behavior is covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- ci-cd
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100