devantler-tech / devantler-tech/ksail

ci harness: default-branch matcher silently skips GitHub `+`/`?` filter patterns

Closed Beginner friendly
#6,508 1 comment 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.