Skip to content

fix(helm): retain GitLab merge-request links in card facts - #26

Merged
geojitsu merged 4 commits into
mainfrom
fm/fm-helm-sync-card-pr-link-multi-host-001
Sep 14, 2026
Merged

geojitsu merged 4 commits into
mainfrom
fm/fm-helm-sync-card-pr-link-multi-host-001

Conversation

@geojitsu

Copy link
Copy Markdown
Owner

Intent

Captain wants a direct link to the PR/MR within the Helm sync card body, in the Facts section, so it's easy to jump to it from the card once a PR is opened.

What Changed

  • Recognize canonical GitHub pull-request and GitLab merge-request URLs when parsing Helm backlog rows, then retain them as PR facts in card bodies.
  • Document the supported URL shapes and add the Helm sync bug record to the documentation audience index.
  • Add coverage for valid GitHub and GitLab links and reject malformed or unsupported merge-request paths.

Risk Assessment

✅ Low: The narrowly scoped parser change preserves canonical GitLab MR links alongside existing GitHub PR links, with behavioral coverage for accepted and rejected URL shapes.

Testing

Completed 1 recorded test check.

  • Outcome: ⚠️ 1 error across 1 run (19m57s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed (2) ✅
  • ⚠️ bin/fm-helm-lib.sh:189 - The new MR alternatives accept /merge_requests/0 and numeric prefixes such as /merge_requests/12x; those are rendered as a PR: Facts link despite not being valid MR URLs. The shared PR parser requires a positive, complete IID. Tighten these new patterns to the same number boundary.
  • ⚠️ bin/fm-helm-lib.sh:189 - Simplification: /merge_requests/<n> is a new bare-GitLab alias, but the existing canonical PR/MR parser accepts only /-/merge_requests/<n> and no intent requirement calls for the alias. Remove this extra acceptance path and its test unless supporting manually supplied noncanonical GitLab links is intended.

🔧 Fix: Tighten Helm PR URL recognition
1 warning still open:

  • ⚠️ docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md:16 - The new bug note says the allowlist accepts bare GitLab /merge_requests/<number> URLs, but the parser accepts only /-/merge_requests/<number> and the amended integration test asserts that bare URLs are omitted. Update the Fix section to name only the two supported canonical shapes.

🔧 Fix: Correct Helm PR-link documentation
✅ Re-checked - no issues remain.

⚠️ **Test** - 1 error
  • 🚨 tests failed with exit code 1
  • bin/fm-test-run.sh --changed --exclude-family real-herdr-gated
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@geojitsu
geojitsu merged commit f718043 into main Sep 14, 2026
14 checks passed
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.

1 participant