From bff69bb0a319649d4b2ef00fb9a79c742c6b1931 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 19:09:04 -0700 Subject: [PATCH] =?UTF-8?q?fix(slug):=20canonicalize=20slash=20branches=20?= =?UTF-8?q?to=20dash=20form=20=E2=80=94=20review=20history=20stops=20split?= =?UTF-8?q?ting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch-name sanitization disagreed across gstack (four incompatible rules), so reviews for the same slash-named branch landed in multiple files and the ship dashboard missed entries. gstack-slug now canonicalizes / to - in one place, and ship's review lookup routes through it; goldens regenerated against the current templates. Fixes #1127, #2550. Contributed by @ShuratCode (PR #2465; duplicate fixes by @xrfael-dev and two others in PRs #1851/#1699/#1621, credited). Co-Authored-By: Claude Fable 5 --- bin/gstack-slug | 2 +- ship/SKILL.md | 21 ++++++++++++--------- ship/SKILL.md.tmpl | 21 ++++++++++++--------- test/fixtures/golden/claude-ship-SKILL.md | 21 ++++++++++++--------- test/fixtures/golden/codex-ship-SKILL.md | 21 ++++++++++++--------- test/fixtures/golden/factory-ship-SKILL.md | 21 ++++++++++++--------- 6 files changed, 61 insertions(+), 46 deletions(-) diff --git a/bin/gstack-slug b/bin/gstack-slug index 24bbca4f1..87d4bb9ba 100755 --- a/bin/gstack-slug +++ b/bin/gstack-slug @@ -49,7 +49,7 @@ if [[ -n "$SLUG" ]]; then fi RAW_BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null) || RAW_BRANCH="" -BRANCH=$(printf '%s' "${RAW_BRANCH:-}" | tr -cd 'a-zA-Z0-9._-') +BRANCH=$(printf '%s' "${RAW_BRANCH:-}" | tr '/' '-' | tr -cd 'a-zA-Z0-9._-') BRANCH="${BRANCH:-unknown}" echo "SLUG=$SLUG" echo "BRANCH=$BRANCH" diff --git a/ship/SKILL.md b/ship/SKILL.md index 4730797b0..d49a20238 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -1350,16 +1350,17 @@ git push -u origin ## Step 20: Persist ship metrics -Log coverage and plan completion data so `/retro` can track trends: +Log coverage and plan completion data so `/retro` can track trends. + +Route the append through `gstack-review-log`. It resolves the project slug and +the canonical branch form itself, creates the directory, validates the JSON, and +enqueues the row for gbrain sync. It takes **no path argument** — never build a +`-reviews.jsonl` path by hand. A branch with a `/` in it turns a +hand-built path into a subdirectory write, and the row goes somewhere `/retro` +will never look. ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)" && mkdir -p ~/.gstack/projects/$SLUG -``` - -Append to `~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl`: - -```bash -echo '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"BRANCH"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"'"$(git rev-parse --abbrev-ref HEAD)"'"}' ``` Substitute from earlier steps: @@ -1368,7 +1369,9 @@ Substitute from earlier steps: - **PLAN_DONE**: count of DONE + CHANGED items from Step 8 (0 if no plan file) - **VERIFY_RESULT**: "pass", "fail", or "skipped" from Step 8.1 - **VERSION**: from the VERSION file -- **BRANCH**: current branch name + +The branch name is filled in by the shell — there is no `BRANCH` placeholder to +substitute. This step is automatic — never skip it, never ask for confirmation. diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index bd50a20c8..c867b020f 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -470,16 +470,17 @@ git push -u origin ## Step 20: Persist ship metrics -Log coverage and plan completion data so `/retro` can track trends: +Log coverage and plan completion data so `/retro` can track trends. + +Route the append through `gstack-review-log`. It resolves the project slug and +the canonical branch form itself, creates the directory, validates the JSON, and +enqueues the row for gbrain sync. It takes **no path argument** — never build a +`-reviews.jsonl` path by hand. A branch with a `/` in it turns a +hand-built path into a subdirectory write, and the row goes somewhere `/retro` +will never look. ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)" && mkdir -p ~/.gstack/projects/$SLUG -``` - -Append to `~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl`: - -```bash -echo '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"BRANCH"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"'"$(git rev-parse --abbrev-ref HEAD)"'"}' ``` Substitute from earlier steps: @@ -488,7 +489,9 @@ Substitute from earlier steps: - **PLAN_DONE**: count of DONE + CHANGED items from Step 8 (0 if no plan file) - **VERIFY_RESULT**: "pass", "fail", or "skipped" from Step 8.1 - **VERSION**: from the VERSION file -- **BRANCH**: current branch name + +The branch name is filled in by the shell — there is no `BRANCH` placeholder to +substitute. This step is automatic — never skip it, never ask for confirmation. diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 4730797b0..d49a20238 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -1350,16 +1350,17 @@ git push -u origin ## Step 20: Persist ship metrics -Log coverage and plan completion data so `/retro` can track trends: +Log coverage and plan completion data so `/retro` can track trends. + +Route the append through `gstack-review-log`. It resolves the project slug and +the canonical branch form itself, creates the directory, validates the JSON, and +enqueues the row for gbrain sync. It takes **no path argument** — never build a +`-reviews.jsonl` path by hand. A branch with a `/` in it turns a +hand-built path into a subdirectory write, and the row goes somewhere `/retro` +will never look. ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)" && mkdir -p ~/.gstack/projects/$SLUG -``` - -Append to `~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl`: - -```bash -echo '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"BRANCH"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"'"$(git rev-parse --abbrev-ref HEAD)"'"}' ``` Substitute from earlier steps: @@ -1368,7 +1369,9 @@ Substitute from earlier steps: - **PLAN_DONE**: count of DONE + CHANGED items from Step 8 (0 if no plan file) - **VERIFY_RESULT**: "pass", "fail", or "skipped" from Step 8.1 - **VERSION**: from the VERSION file -- **BRANCH**: current branch name + +The branch name is filled in by the shell — there is no `BRANCH` placeholder to +substitute. This step is automatic — never skip it, never ask for confirmation. diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index 4ac024a20..174b78598 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -2736,16 +2736,17 @@ Print the branch name, remote URL, and instruct the user to create the PR/MR man ## Step 20: Persist ship metrics -Log coverage and plan completion data so `/retro` can track trends: +Log coverage and plan completion data so `/retro` can track trends. + +Route the append through `gstack-review-log`. It resolves the project slug and +the canonical branch form itself, creates the directory, validates the JSON, and +enqueues the row for gbrain sync. It takes **no path argument** — never build a +`-reviews.jsonl` path by hand. A branch with a `/` in it turns a +hand-built path into a subdirectory write, and the row goes somewhere `/retro` +will never look. ```bash -eval "$($GSTACK_ROOT/bin/gstack-slug 2>/dev/null)" && mkdir -p ~/.gstack/projects/$SLUG -``` - -Append to `~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl`: - -```bash -echo '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"BRANCH"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +$GSTACK_ROOT/bin/gstack-review-log '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"'"$(git rev-parse --abbrev-ref HEAD)"'"}' ``` Substitute from earlier steps: @@ -2754,7 +2755,9 @@ Substitute from earlier steps: - **PLAN_DONE**: count of DONE + CHANGED items from Step 8 (0 if no plan file) - **VERIFY_RESULT**: "pass", "fail", or "skipped" from Step 8.1 - **VERSION**: from the VERSION file -- **BRANCH**: current branch name + +The branch name is filled in by the shell — there is no `BRANCH` placeholder to +substitute. This step is automatic — never skip it, never ask for confirmation. diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index cb96693bb..ebfcca8cd 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -3142,16 +3142,17 @@ Print the branch name, remote URL, and instruct the user to create the PR/MR man ## Step 20: Persist ship metrics -Log coverage and plan completion data so `/retro` can track trends: +Log coverage and plan completion data so `/retro` can track trends. + +Route the append through `gstack-review-log`. It resolves the project slug and +the canonical branch form itself, creates the directory, validates the JSON, and +enqueues the row for gbrain sync. It takes **no path argument** — never build a +`-reviews.jsonl` path by hand. A branch with a `/` in it turns a +hand-built path into a subdirectory write, and the row goes somewhere `/retro` +will never look. ```bash -eval "$($GSTACK_ROOT/bin/gstack-slug 2>/dev/null)" && mkdir -p ~/.gstack/projects/$SLUG -``` - -Append to `~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl`: - -```bash -echo '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"BRANCH"}' >> ~/.gstack/projects/$SLUG/$BRANCH-reviews.jsonl +$GSTACK_ROOT/bin/gstack-review-log '{"skill":"ship","timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","coverage_pct":COVERAGE_PCT,"plan_items_total":PLAN_TOTAL,"plan_items_done":PLAN_DONE,"verification_result":"VERIFY_RESULT","version":"VERSION","branch":"'"$(git rev-parse --abbrev-ref HEAD)"'"}' ``` Substitute from earlier steps: @@ -3160,7 +3161,9 @@ Substitute from earlier steps: - **PLAN_DONE**: count of DONE + CHANGED items from Step 8 (0 if no plan file) - **VERIFY_RESULT**: "pass", "fail", or "skipped" from Step 8.1 - **VERSION**: from the VERSION file -- **BRANCH**: current branch name + +The branch name is filled in by the shell — there is no `BRANCH` placeholder to +substitute. This step is automatic — never skip it, never ask for confirmation.