From 10148d70bc5ee1e76c7da6924a51c58c405f6682 Mon Sep 17 00:00:00 2001 From: firstmate-crewmate Date: Mon, 14 Sep 2026 03:09:32 +0000 Subject: [PATCH 1/4] fix(helm): preserve PR links from GitLab --- bin/fm-helm-lib.sh | 2 +- ...2026-09-14-helm-sync-multi-host-pr-link.md | 20 +++++++++++ tests/fm-helm-sync.test.sh | 35 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md diff --git a/bin/fm-helm-lib.sh b/bin/fm-helm-lib.sh index 15bbf029ea4..2c73b306d04 100755 --- a/bin/fm-helm-lib.sh +++ b/bin/fm-helm-lib.sh @@ -186,7 +186,7 @@ fm_helm_backlog_parse_program() { since:metadata_word($m.rest; "since"), merged:metadata_word($m.rest; "merged"), reported:metadata_word($m.rest; "reported"), done:metadata_word($m.rest; "done"), blocked_by_ids:blocked_by_ids($m.rest), - pr_url:(([$m.rest | scan(url_pattern)] | map(select(test("/pull/[0-9]+"))) | .[0]) // null), + pr_url:(([$m.rest | scan(url_pattern)] | map(select(test("/pull/[0-9]+") or test("/-/merge_requests/[0-9]+") or test("/merge_requests/[0-9]+"))) | .[0]) // null), report_path:cap($m.rest; ".*(?data/[^[:space:])]+/report\\.md).*"), body_lines:[]}] elif ($line | test("^[[:space:]]+")) and (.records | length) > 0 and .records[-1].structured then diff --git a/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md b/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md new file mode 100644 index 00000000000..4c315fbd8b6 --- /dev/null +++ b/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md @@ -0,0 +1,20 @@ +--- +title: Helm sync dropped non-GitHub PR links +description: Helm sync now preserves GitHub pull-request and GitLab merge-request URLs in card Facts sections. +--- + +## Symptom + +Helm cards showed a `PR:` Facts line for GitHub pull-request URLs, but omitted the line for GitLab merge-request URLs even when `fm-helm-pr-check` had recorded the link in the backlog. + +## Root cause + +The backlog parser selected a PR URL only when it matched the GitHub-specific `/pull/` path. GitLab's `/-/merge_requests/` path therefore passed through as an ordinary backlog URL and was removed from the rendered card title without being retained for the Facts section. + +## Fix + +The parser now uses a short explicit allowlist for GitHub `/pull/`, GitLab `/-/merge_requests/`, and GitLab `/merge_requests/` paths. The existing card renderer continues to place a retained link in the Facts section. + +## Prevention + +`tests/fm-helm-sync.test.sh` creates cards from backlog rows containing both GitHub and GitLab links and asserts that each appears as a `PR:` Facts line. Keep new provider-specific URL shapes in this allowlist rather than accepting every URL from task notes. diff --git a/tests/fm-helm-sync.test.sh b/tests/fm-helm-sync.test.sh index 7df806f1498..72b86869fa9 100644 --- a/tests/fm-helm-sync.test.sh +++ b/tests/fm-helm-sync.test.sh @@ -268,6 +268,41 @@ run_watch() { # "$WATCH" } +# --------------------------------------------------------------------------- +# PR links: card Facts retain supported GitHub and GitLab link shapes. +# --------------------------------------------------------------------------- +case_dir="$TMP_ROOT/pr-link-shapes" +mkdir -p "$case_dir/home/config" "$case_dir/home/data" "$case_dir/home/state" +fb=$(install_fakes "$case_dir") +printf '{"owner":"geojitsu","number":2}\n' > "$case_dir/home/config/helm.json" +cat > "$case_dir/home/data/backlog.md" <<'EOF' +# Backlog + +## Queued +- [ ] github-pr-task - GitHub PR task (repo: firstmate) (kind: ship) (since: 2026-09-05) https://github.com/geojitsu/firstmate/pull/123 +- [ ] gitlab-mr-task - GitLab MR task (repo: nocout) (kind: ship) (since: 2026-09-05) https://gitlab.com/dc-noc/nocout/-/merge_requests/123 +- [ ] gitlab-bare-mr-task - GitLab bare MR task (repo: nocout) (kind: ship) (since: 2026-09-05) https://gitlab.com/dc-noc/nocout/merge_requests/456 +## Done +EOF +board_json '[]' > "$case_dir/board.json" +run_sync "$case_dir" "$fb" >/dev/null 2>&1 || fail "PR-link shape sync failed" +jq -e --arg url 'https://github.com/geojitsu/firstmate/pull/123' ' + [.data.user.projectV2.items.nodes[].content.body] + | any(.[]; contains("- **PR:** " + $url)) +' "$case_dir/board-state.json" >/dev/null \ + || fail "a GitHub pull-request URL did not render in the Facts section" +jq -e --arg url 'https://gitlab.com/dc-noc/nocout/-/merge_requests/123' ' + [.data.user.projectV2.items.nodes[].content.body] + | any(.[]; contains("- **PR:** " + $url)) +' "$case_dir/board-state.json" >/dev/null \ + || fail "a GitLab merge-request URL did not render in the Facts section" +jq -e --arg url 'https://gitlab.com/dc-noc/nocout/merge_requests/456' ' + [.data.user.projectV2.items.nodes[].content.body] + | any(.[]; contains("- **PR:** " + $url)) +' "$case_dir/board-state.json" >/dev/null \ + || fail "a bare GitLab merge-request URL did not render in the Facts section" +pass "supported GitHub and GitLab PR links render in card Facts" + # --------------------------------------------------------------------------- # Two-home union: close-missing only fires for a card in no home's backlog. # --------------------------------------------------------------------------- From 08cb28e3be09f77018bf0f8c00917f4ab5473aab Mon Sep 17 00:00:00 2001 From: firstmate-crewmate Date: Mon, 14 Sep 2026 03:18:57 +0000 Subject: [PATCH 2/4] no-mistakes(review): Tighten Helm PR URL recognition --- bin/fm-helm-lib.sh | 2 +- tests/fm-helm-sync.test.sh | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/bin/fm-helm-lib.sh b/bin/fm-helm-lib.sh index 2c73b306d04..a9444dc6e1a 100755 --- a/bin/fm-helm-lib.sh +++ b/bin/fm-helm-lib.sh @@ -186,7 +186,7 @@ fm_helm_backlog_parse_program() { since:metadata_word($m.rest; "since"), merged:metadata_word($m.rest; "merged"), reported:metadata_word($m.rest; "reported"), done:metadata_word($m.rest; "done"), blocked_by_ids:blocked_by_ids($m.rest), - pr_url:(([$m.rest | scan(url_pattern)] | map(select(test("/pull/[0-9]+") or test("/-/merge_requests/[0-9]+") or test("/merge_requests/[0-9]+"))) | .[0]) // null), + pr_url:(([$m.rest | scan(url_pattern)] | map(select(test("/pull/[1-9][0-9]*$") or test("/-/merge_requests/[1-9][0-9]*$"))) | .[0]) // null), report_path:cap($m.rest; ".*(?data/[^[:space:])]+/report\\.md).*"), body_lines:[]}] elif ($line | test("^[[:space:]]+")) and (.records | length) > 0 and .records[-1].structured then diff --git a/tests/fm-helm-sync.test.sh b/tests/fm-helm-sync.test.sh index 72b86869fa9..f585cbfcb79 100644 --- a/tests/fm-helm-sync.test.sh +++ b/tests/fm-helm-sync.test.sh @@ -269,7 +269,7 @@ run_watch() { # } # --------------------------------------------------------------------------- -# PR links: card Facts retain supported GitHub and GitLab link shapes. +# PR links: card Facts retain canonical GitHub and GitLab link shapes. # --------------------------------------------------------------------------- case_dir="$TMP_ROOT/pr-link-shapes" mkdir -p "$case_dir/home/config" "$case_dir/home/data" "$case_dir/home/state" @@ -282,6 +282,8 @@ cat > "$case_dir/home/data/backlog.md" <<'EOF' - [ ] github-pr-task - GitHub PR task (repo: firstmate) (kind: ship) (since: 2026-09-05) https://github.com/geojitsu/firstmate/pull/123 - [ ] gitlab-mr-task - GitLab MR task (repo: nocout) (kind: ship) (since: 2026-09-05) https://gitlab.com/dc-noc/nocout/-/merge_requests/123 - [ ] gitlab-bare-mr-task - GitLab bare MR task (repo: nocout) (kind: ship) (since: 2026-09-05) https://gitlab.com/dc-noc/nocout/merge_requests/456 +- [ ] gitlab-zero-mr-task - GitLab zero MR task (repo: nocout) (kind: ship) (since: 2026-09-05) https://gitlab.com/dc-noc/nocout/-/merge_requests/0 +- [ ] gitlab-suffixed-mr-task - GitLab suffixed MR task (repo: nocout) (kind: ship) (since: 2026-09-05) https://gitlab.com/dc-noc/nocout/-/merge_requests/12x ## Done EOF board_json '[]' > "$case_dir/board.json" @@ -298,10 +300,20 @@ jq -e --arg url 'https://gitlab.com/dc-noc/nocout/-/merge_requests/123' ' || fail "a GitLab merge-request URL did not render in the Facts section" jq -e --arg url 'https://gitlab.com/dc-noc/nocout/merge_requests/456' ' [.data.user.projectV2.items.nodes[].content.body] - | any(.[]; contains("- **PR:** " + $url)) + | all(.[]; contains("- **PR:** " + $url) | not) +' "$case_dir/board-state.json" >/dev/null \ + || fail "a bare GitLab merge-request URL rendered in the Facts section" +jq -e --arg url 'https://gitlab.com/dc-noc/nocout/-/merge_requests/0' ' + [.data.user.projectV2.items.nodes[].content.body] + | all(.[]; contains("- **PR:** " + $url) | not) +' "$case_dir/board-state.json" >/dev/null \ + || fail "a zero GitLab merge-request URL rendered in the Facts section" +jq -e --arg url 'https://gitlab.com/dc-noc/nocout/-/merge_requests/12x' ' + [.data.user.projectV2.items.nodes[].content.body] + | all(.[]; contains("- **PR:** " + $url) | not) ' "$case_dir/board-state.json" >/dev/null \ - || fail "a bare GitLab merge-request URL did not render in the Facts section" -pass "supported GitHub and GitLab PR links render in card Facts" + || fail "a suffixed GitLab merge-request URL rendered in the Facts section" +pass "canonical GitHub and GitLab PR links render in card Facts" # --------------------------------------------------------------------------- # Two-home union: close-missing only fires for a card in no home's backlog. From 88c536d97aac2a0aef9ef88912989fd4d0e7f3a8 Mon Sep 17 00:00:00 2001 From: firstmate-crewmate Date: Mon, 14 Sep 2026 03:21:19 +0000 Subject: [PATCH 3/4] no-mistakes(review): Correct Helm PR-link documentation --- docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md b/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md index 4c315fbd8b6..65fc3198189 100644 --- a/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md +++ b/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md @@ -13,7 +13,7 @@ The backlog parser selected a PR URL only when it matched the GitHub-specific `/ ## Fix -The parser now uses a short explicit allowlist for GitHub `/pull/`, GitLab `/-/merge_requests/`, and GitLab `/merge_requests/` paths. The existing card renderer continues to place a retained link in the Facts section. +The parser now uses a short explicit allowlist for GitHub `/pull/` and GitLab `/-/merge_requests/` paths. The existing card renderer continues to place a retained link in the Facts section. ## Prevention From 71d3531d7e6f08ad70d1d369440357a41610e108 Mon Sep 17 00:00:00 2001 From: firstmate-crewmate Date: Mon, 14 Sep 2026 03:45:37 +0000 Subject: [PATCH 4/4] no-mistakes(document): Document canonical Helm PR/MR card links --- docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md | 2 +- docs/configuration.md | 1 + docs/documentation-audiences.json | 4 ++++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md b/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md index 65fc3198189..53eb4319fcf 100644 --- a/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md +++ b/docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md @@ -13,7 +13,7 @@ The backlog parser selected a PR URL only when it matched the GitHub-specific `/ ## Fix -The parser now uses a short explicit allowlist for GitHub `/pull/` and GitLab `/-/merge_requests/` paths. The existing card renderer continues to place a retained link in the Facts section. +The parser now recognizes the canonical PR and MR URL shapes documented in [Helm board sync configuration](../configuration.md#helm-board-sync-confighelmjson). The existing card renderer continues to place a retained link in the Facts section. ## Prevention diff --git a/docs/configuration.md b/docs/configuration.md index d5f43d1580f..992deda60df 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -144,6 +144,7 @@ An external interruption, including the watcher's check timeout, can end without A `--force` run that reaches the partial path leaves `state/.helm-sync-resume` so the next run is forced too, which keeps the captain's board edits on the cards it never reached on the reconciliation path instead of the backlog-wins path. Field authority: `data/backlog.md` in the owning home is authoritative for a card's title, body, kind, repository, priority, and lifecycle status. +When a backlog row contains a GitHub `/pull/` or GitLab `/-/merge_requests/` URL, the card body includes it as a `PR` fact. The board is authoritative only for the captain's own edits, only for Priority, Status, and card text, and only on an explicit `bin/fm-helm-sync.sh --force` read. On `--force` a captain edit to a card's Priority is written back into the owning backlog row; a move into the dispatch status raises one durable dispatch `check` wake for ordinary firstmate intake; and a move to Done on a live task, a move backwards, a title or body edit, a new captain card, or a deleted card each raise one `check` wake and change no backlog task mechanically. The sync compares Status, Priority, title, and body with their own recorded board baselines. diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index b8d3a91c758..e615e8dee0c 100644 --- a/docs/documentation-audiences.json +++ b/docs/documentation-audiences.json @@ -492,6 +492,10 @@ "path": "docs/bugs/2026-09-13-helm-sync-deadline-lost-progress.md", "audience": "maintainer-architecture" }, + { + "path": "docs/bugs/2026-09-14-helm-sync-multi-host-pr-link.md", + "audience": "maintainer-architecture" + }, { "path": "docs/calm-mode-feasibility.md", "audience": "maintainer-verification"