Skip to content

fix(attest jira): match Jira keys case-insensitively by uppercasing commit text#971

Open
jbrejner wants to merge 2 commits into
mainfrom
fix/jira-mixed-case-key-matching
Open

fix(attest jira): match Jira keys case-insensitively by uppercasing commit text#971
jbrejner wants to merge 2 commits into
mainfrom
fix/jira-mixed-case-key-matching

Conversation

@jbrejner

Copy link
Copy Markdown
Contributor

Summary

  • FindJiraIssueKeys was silently missing Jira keys written in mixed case (e.g. tDIE-12419) because the regex was applied to the original commit text
  • Fix: uppercase the full input before matching so all case variants are found and returned in canonical uppercase form (which Jira's API accepts)
  • Also adds \b word boundary anchors and documents the case-insensitive behaviour in attestJira help text

Changes

  • internal/jira/jira.goFindJiraIssueKeys uppercases input before regex; MakeJiraIssueKeyPattern uppercases caller-supplied project keys and adds \b boundary anchor
  • internal/jira/jira_test.go — new test cases covering mixed-case keys
  • cmd/kosli/attestJira.go — help text updated to document case-insensitive matching and the note-1/PROJ-42 false-positive corner case

Test plan

  • make test_integration_single TARGET=AttestJiraSuite passes
  • make lint passes
  • kosli attest jira --help shows updated case-insensitive matching documentation

🤖 Generated with Claude Code

…ommit text

Previously `FindJiraIssueKeys` applied the regex to the original commit
message, so mixed-case keys like `tDIE-12419` were silently missed.
The fix uppercases the full input text before matching, meaning all
returned keys are in canonical uppercase form (which Jira's API accepts
regardless of the original casing).

- `FindJiraIssueKeys`: uppercase text before regex, dedup and CVE filter
  on the uppercased copy so all case variants collapse into one result
- `MakeJiraIssueKeyPattern`: add `\b` word boundary anchor, uppercase
  caller-supplied project keys for consistency
- `attestJira` help text: document case-insensitive matching and the
  `note-1`/`PROJ-42` false-positive corner case

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jbrejner's task in 2m 25s —— View job


Review: case-insensitive Jira key matching

I reviewed the diff against origin/main. Static analysis is clean and the implementation is solid and well-tested.

  • Gather context & read changed files
  • Static analysis (go build ./..., go vet ./internal/jira/...) — both pass
  • Review internal/jira/jira.go logic
  • Review tests & help text
  • Post review

✅ What's good

  • The fix is the right approach: uppercasing the input and applying an uppercase-only pattern is simpler and more robust than embedding (?i) or duplicating case variants in the regex. Canonical uppercase output matches what Jira's API accepts.
  • Careful internal consistencyisPartialMultiSegment operates on upperText throughout (not a mix of original + uppercased). This avoids byte-offset bugs from Unicode case-folding (strings.ToUpper can change byte length, e.g. ßSS), since all indexing stays on one string.
  • Excellent test coverage. The tricky ordering case tdie-12419-5 foo TDIE-12419 (a CVE-like lowercase occurrence appearing before a valid standalone uppercase one) is exactly the kind of edge case that's easy to miss — good that it's pinned.
  • Adding \b tightens the pattern, and uppercasing caller-supplied project keys removes a latent footgun (a lowercase --jira-project-key previously matched nothing).

🟡 Worth considering (non-blocking)

  1. Precision tradeoff (see inline comment on attestJira.go): case-insensitive matching now promotes common lowercase tokens like utf-8/sha-1/iso-8601 into Jira-key candidates that previously only matched when uppercase. These are single-segment, so the multi-segment CVE- filter doesn't catch them. This is inherent to case-insensitive matching, is documented, and is mitigated by --jira-project-key — so it's a heads-up, not a defect. Suggestion: use one of those realistic tokens as the help-text example instead of note-1.
  2. Minor / optionalFindJiraIssueKeys recompiles both regexes (MakeJiraIssueKeyPattern and the ^-\d dashDigit) on every call. The dashDigit pattern is constant and could be a package-level var. Negligible for commit-message-sized input and consistent with existing style, so only worth it if this is ever called in a hot path.

Verdict

The change correctly fixes the reported bug (mixed-case keys like tDIE-12419 were silently missed), is well-documented, and the tests are comprehensive. No required changes — only the two optional notes above.

@jbrejner jbrejner requested a review from tooky June 24, 2026 14:32
Comment thread internal/jira/jira.go
Comment thread cmd/kosli/attestJira.go
@mbevc1

mbevc1 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

@jbrejner looks good, just a minor nit / optional:
dashDigit := regexp.MustCompile(\^-\d) recompiles on every call (jira.go:141). It's constant — could be a package-levelvar. Negligible for commit-sized input; only matters if this ever hits a hot path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants