From ab8cfcc91ba2c9c9117eabec0bfa399355d796f2 Mon Sep 17 00:00:00 2001 From: Chai Bot Date: Wed, 9 Sep 2026 15:39:52 +0000 Subject: [PATCH 1/6] Extract deterministic publish operations into shared script Add _shared/scripts/publish.sh with subcommands for pre-flight checks, push, existing PR/MR detection, GitHub PR creation, GitLab MR creation, and metadata serialization. Update all six publish/PR phase skills (bugfix, implement, e2e, prd, design, docs-writer) to call the shared script for deterministic operations while keeping AI-dependent work (PR body generation, cross-cutting review, user prompts) in the skills. The script is shellcheck-clean, supports both GitHub (gh) and GitLab (glab) platforms, and uses structured exit codes so skills can detect specific failure modes (auth, push, PR creation) and apply the appropriate fallback strategy. PATCH bump: bugfix 0.8.1, implement 0.9.1, e2e 0.7.1, prd 0.9.2, design 0.9.2, docs-writer 0.3.2 --- AGENTS.md | 4 +- _shared/scripts/publish.sh | 499 ++++++++++++++++++++++++++++++++ bugfix/SKILL.md | 2 +- bugfix/skills/pr.md | 136 ++++----- design/SKILL.md | 2 +- design/skills/publish.md | 72 +++-- docs-writer/SKILL.md | 2 +- docs-writer/skills/create-mr.md | 86 +++--- e2e/SKILL.md | 2 +- e2e/skills/publish.md | 103 ++++--- implement/SKILL.md | 2 +- implement/skills/publish.md | 103 ++++--- prd/SKILL.md | 2 +- prd/skills/publish.md | 58 ++-- 14 files changed, 834 insertions(+), 239 deletions(-) create mode 100755 _shared/scripts/publish.sh diff --git a/AGENTS.md b/AGENTS.md index 2a035646..da07d1c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,6 +83,7 @@ _shared/ sizing-rubric.md # Shared sizing definitions (T-shirt sizes, heuristics, team effort guidance) scripts/ provenance.py # Capture/render CLI (used by prd and design provenance recipes) + publish.sh # Deterministic publish ops (preflight, push, PR/MR creation, metadata) recipes/ capture-provenance-event.md # Append session-local provenance on doc-mutating phases phase-override-resolution.md # Project-level phase override lookup and activation @@ -196,7 +197,8 @@ ai-workflows/ │ ├── review-protocol.md # Shared code review criteria and finding format │ ├── sizing-rubric.md # Shared sizing definitions and heuristics │ ├── scripts/ -│ │ └── provenance.py # Capture/render CLI for prd/design provenance +│ │ ├── provenance.py # Capture/render CLI for prd/design provenance +│ │ └── publish.sh # Deterministic publish ops (push, PR/MR, metadata) │ └── recipes/ │ ├── capture-provenance-event.md │ ├── phase-override-resolution.md # Project-level phase override lookup diff --git a/_shared/scripts/publish.sh b/_shared/scripts/publish.sh new file mode 100755 index 00000000..9584215f --- /dev/null +++ b/_shared/scripts/publish.sh @@ -0,0 +1,499 @@ +#!/usr/bin/env bash +# Deterministic publish operations for ai-workflows. +# +# Provides reusable subcommands for the publish/PR/MR phase of multiple +# workflows (bugfix, implement, e2e, prd, design, docs-writer). Each +# subcommand handles one discrete, deterministic operation — the calling +# skill file retains ownership of AI-dependent work (PR body generation, +# cross-cutting review, user confirmation prompts). +# +# Subcommands: +# preflight Pre-flight checks (auth, branch, uncommitted changes) +# push Push a branch to a remote +# check-existing Check whether a PR/MR already exists for a branch +# create-pr Create a GitHub pull request via gh CLI +# create-mr Create a GitLab merge request via glab CLI +# save-metadata Write publish-metadata.json +# +# Usage: +# publish.sh preflight [--platform github|gitlab] +# publish.sh push --remote --branch +# publish.sh check-existing --repo --head [--platform github|gitlab] +# publish.sh create-pr --repo --base --head \ +# --title [--body-file <path>] [--body <text>] [--draft] [--labels <csv>] +# publish.sh create-mr --project <path> --source <branch> --target <branch> \ +# --title <title> [--description <text>] [--draft] +# publish.sh save-metadata --file <path> [key=value ...] +# +# Exit codes: +# 0 — success +# 1 — missing argument or configuration error +# 2 — pre-flight check failed (auth, branch, or changes issue) +# 3 — push failed +# 4 — PR/MR creation failed +# 5 — existing PR/MR found (check-existing only; prints details on stdout) + +set -euo pipefail + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +fail() { + printf 'ERROR: %s\n' "$1" >&2 + exit "${2:-1}" +} + +info() { + printf 'INFO: %s\n' "$1" >&2 +} + +usage() { + sed -n '/^# Usage:/,/^# Exit codes:/{ /^# Exit codes:/d; s/^# \?//; p }' "$0" >&2 + exit 1 +} + +require_arg() { + if [[ -z "${2:-}" ]]; then + fail "Missing required argument: $1" 1 + fi +} + +# --------------------------------------------------------------------------- +# Subcommand: preflight +# --------------------------------------------------------------------------- +# Checks authentication (gh or glab), current branch, and uncommitted +# changes. Prints a structured status block on stdout for the calling +# skill to parse. +# +# Flags: +# --platform github|gitlab Which CLI to check (default: github) + +cmd_preflight() { + local platform="github" + + while [[ $# -gt 0 ]]; do + case "$1" in + --platform) platform="$2"; shift 2 ;; + *) fail "preflight: unknown flag: $1" 1 ;; + esac + done + + local auth_ok="false" + local auth_user="" + local branch="" + local has_uncommitted="false" + local has_staged="false" + + # -- Auth check -- + case "$platform" in + github) + if gh auth status >/dev/null 2>&1; then + auth_ok="true" + auth_user=$(gh api user --jq .login 2>/dev/null || true) + if [[ -z "$auth_user" ]]; then + # GitHub App / bot — try installation endpoint + auth_user=$(gh api /installation/repositories \ + --jq '.repositories[0].owner.login' 2>/dev/null || true) + fi + fi + ;; + gitlab) + if glab auth status >/dev/null 2>&1; then + auth_ok="true" + auth_user=$(glab api user --jq .username 2>/dev/null || true) + fi + ;; + *) fail "preflight: invalid platform: $platform (expected github or gitlab)" 1 ;; + esac + + # -- Branch -- + branch=$(git branch --show-current 2>/dev/null || true) + + # -- Uncommitted changes -- + if ! git diff --quiet 2>/dev/null; then + has_uncommitted="true" + fi + if ! git diff --cached --quiet 2>/dev/null; then + has_staged="true" + fi + + # -- Output structured block -- + cat <<EOF +auth_ok=${auth_ok} +auth_user=${auth_user} +branch=${branch} +has_uncommitted=${has_uncommitted} +has_staged=${has_staged} +platform=${platform} +EOF +} + +# --------------------------------------------------------------------------- +# Subcommand: push +# --------------------------------------------------------------------------- +# Pushes a branch to the specified remote with -u (set upstream). +# +# Flags: +# --remote <name> Git remote name (e.g., fork, origin) +# --branch <branch> Branch name to push + +cmd_push() { + local remote="" + local branch="" + + while [[ $# -gt 0 ]]; do + case "$1" in + --remote) remote="$2"; shift 2 ;; + --branch) branch="$2"; shift 2 ;; + *) fail "push: unknown flag: $1" 1 ;; + esac + done + + require_arg "--remote" "$remote" + require_arg "--branch" "$branch" + + # Verify the remote exists + if ! git remote get-url "$remote" >/dev/null 2>&1; then + fail "push: remote '$remote' does not exist. Available remotes: $(git remote | tr '\n' ' ')" 3 + fi + + info "Pushing $branch to $remote..." + if ! git push -u "$remote" "$branch" 2>&1; then + fail "push: git push failed (remote=$remote, branch=$branch)" 3 + fi + + info "Push successful: $remote/$branch" +} + +# --------------------------------------------------------------------------- +# Subcommand: check-existing +# --------------------------------------------------------------------------- +# Checks whether a PR (GitHub) or MR (GitLab) already exists for the +# given branch. Prints the PR/MR number and URL on stdout if found. +# +# Flags: +# --repo <owner/repo> Target repository +# --head <ref> Branch or owner:branch to match +# --platform github|gitlab Which platform (default: github) +# +# Exit code 0 if NO existing PR/MR found (safe to create one). +# Exit code 5 if an existing PR/MR IS found (details on stdout). + +cmd_check_existing() { + local repo="" + local head="" + local platform="github" + + while [[ $# -gt 0 ]]; do + case "$1" in + --repo) repo="$2"; shift 2 ;; + --head) head="$2"; shift 2 ;; + --platform) platform="$2"; shift 2 ;; + *) fail "check-existing: unknown flag: $1" 1 ;; + esac + done + + require_arg "--repo" "$repo" + require_arg "--head" "$head" + + case "$platform" in + github) + local result + result=$(gh pr list --repo "$repo" --head "$head" \ + --json number,url --jq '.[0] // empty' 2>/dev/null || true) + if [[ -n "$result" ]]; then + echo "$result" + exit 5 + fi + ;; + gitlab) + local source_branch="$head" + local result + result=$(glab mr list --repo "$repo" --source-branch "$source_branch" \ + --json iid,web_url --jq '.[0] // empty' 2>/dev/null || true) + if [[ -n "$result" ]]; then + echo "$result" + exit 5 + fi + ;; + *) fail "check-existing: invalid platform: $platform" 1 ;; + esac + + # No existing PR/MR found + info "No existing PR/MR found for head=$head on $repo" +} + +# --------------------------------------------------------------------------- +# Subcommand: create-pr +# --------------------------------------------------------------------------- +# Creates a GitHub pull request via gh CLI. +# +# Flags: +# --repo <owner/repo> Target repository (required for fork-based PRs) +# --base <branch> Base branch (e.g., main) +# --head <ref> Head ref — owner:branch for forks, branch for direct +# --title <title> PR title +# --body-file <path> Path to file containing PR body (mutually exclusive with --body) +# --body <text> Inline PR body text (mutually exclusive with --body-file) +# --draft Create as draft PR (default: true) +# --no-draft Create as non-draft PR +# --labels <csv> Comma-separated label names +# +# On success, prints the PR URL on stdout. + +cmd_create_pr() { + local repo="" + local base="" + local head="" + local title="" + local body_file="" + local body="" + local draft="true" + local labels="" + + while [[ $# -gt 0 ]]; do + case "$1" in + --repo) repo="$2"; shift 2 ;; + --base) base="$2"; shift 2 ;; + --head) head="$2"; shift 2 ;; + --title) title="$2"; shift 2 ;; + --body-file) body_file="$2"; shift 2 ;; + --body) body="$2"; shift 2 ;; + --draft) draft="true"; shift ;; + --no-draft) draft="false"; shift ;; + --labels) labels="$2"; shift 2 ;; + *) fail "create-pr: unknown flag: $1" 1 ;; + esac + done + + require_arg "--base" "$base" + require_arg "--head" "$head" + require_arg "--title" "$title" + + # Build gh pr create command + local -a cmd=(gh pr create) + + if [[ "$draft" == "true" ]]; then + cmd+=(--draft) + fi + + if [[ -n "$repo" ]]; then + cmd+=(--repo "$repo") + fi + + cmd+=(--base "$base" --head "$head" --title "$title") + + if [[ -n "$body_file" ]]; then + if [[ ! -f "$body_file" ]]; then + fail "create-pr: body file not found: $body_file" 1 + fi + cmd+=(--body-file "$body_file") + elif [[ -n "$body" ]]; then + cmd+=(--body "$body") + else + cmd+=(--body "") + fi + + if [[ -n "$labels" ]]; then + cmd+=(--label "$labels") + fi + + info "Creating PR: ${title}" + local pr_url + if pr_url=$("${cmd[@]}" 2>&1); then + # gh pr create prints the URL on success + echo "$pr_url" + info "PR created: $pr_url" + else + # Print the error but use a distinct exit code so the skill can + # detect the failure and fall back (e.g., to a compare URL). + printf '%s\n' "$pr_url" >&2 + exit 4 + fi +} + +# --------------------------------------------------------------------------- +# Subcommand: create-mr +# --------------------------------------------------------------------------- +# Creates a GitLab merge request via glab CLI. +# +# Flags: +# --project <path> Upstream project path (for fork-based MRs) +# --source <branch> Source branch +# --target <branch> Target branch (e.g., main) +# --title <title> MR title +# --description <text> MR description text +# --desc-file <path> Path to file containing MR description +# --draft Create as draft MR (default: true) +# --no-draft Create as non-draft MR +# --head <project> Fork project path (for fork-based MRs) +# +# On success, prints the MR URL on stdout. + +cmd_create_mr() { + local project="" + local source_branch="" + local target_branch="" + local title="" + local description="" + local desc_file="" + local draft="true" + local head_project="" + + while [[ $# -gt 0 ]]; do + case "$1" in + --project) project="$2"; shift 2 ;; + --source) source_branch="$2"; shift 2 ;; + --target) target_branch="$2"; shift 2 ;; + --title) title="$2"; shift 2 ;; + --description) description="$2"; shift 2 ;; + --desc-file) desc_file="$2"; shift 2 ;; + --draft) draft="true"; shift ;; + --no-draft) draft="false"; shift ;; + --head) head_project="$2"; shift 2 ;; + *) fail "create-mr: unknown flag: $1" 1 ;; + esac + done + + require_arg "--source" "$source_branch" + require_arg "--target" "$target_branch" + require_arg "--title" "$title" + + # Build glab mr create command + local -a cmd=(glab mr create --yes) + + if [[ "$draft" == "true" ]]; then + cmd+=(--draft) + fi + + if [[ -n "$project" ]]; then + cmd+=(--repo "$project") + fi + + if [[ -n "$head_project" ]]; then + cmd+=(--head "$head_project") + fi + + cmd+=(--source-branch "$source_branch" --target-branch "$target_branch" --title "$title") + + if [[ -n "$desc_file" ]]; then + if [[ ! -f "$desc_file" ]]; then + fail "create-mr: description file not found: $desc_file" 1 + fi + # glab uses --description, not --body-file — read the file content + description=$(<"$desc_file") + fi + + if [[ -n "$description" ]]; then + cmd+=(--description "$description") + fi + + info "Creating MR: ${title}" + local mr_url + if mr_url=$("${cmd[@]}" 2>&1); then + echo "$mr_url" + info "MR created: $mr_url" + else + printf '%s\n' "$mr_url" >&2 + exit 4 + fi +} + +# --------------------------------------------------------------------------- +# Subcommand: save-metadata +# --------------------------------------------------------------------------- +# Writes a JSON metadata file from key=value pairs. +# +# Flags: +# --file <path> Output file path (required) +# +# Remaining positional arguments are key=value pairs. Values that look +# like integers are stored as JSON numbers; everything else is a JSON +# string. +# +# Example: +# publish.sh save-metadata --file .artifacts/impl/EDM-1/publish-metadata.json \ +# repo=acme/project branch=feat/x base=main pr_number=42 \ +# pr_url=https://github.com/acme/project/pull/42 jira_key=EDM-1 + +cmd_save_metadata() { + local file="" + local -a pairs=() + + while [[ $# -gt 0 ]]; do + case "$1" in + --file) file="$2"; shift 2 ;; + *=*) pairs+=("$1"); shift ;; + *) fail "save-metadata: unexpected argument: $1 (expected key=value)" 1 ;; + esac + done + + require_arg "--file" "$file" + + if [[ ${#pairs[@]} -eq 0 ]]; then + fail "save-metadata: no key=value pairs provided" 1 + fi + + # Build JSON using printf — avoids jq dependency. + # Keys are sorted alphabetically for stable output. + local json="{" + local first="true" + local -a sorted_pairs + IFS=$'\n' read -r -d '' -a sorted_pairs < <(printf '%s\n' "${pairs[@]}" | sort && printf '\0') || true + + for pair in "${sorted_pairs[@]}"; do + local key="${pair%%=*}" + local value="${pair#*=}" + + if [[ "$first" == "true" ]]; then + first="false" + else + json+="," + fi + + # Determine JSON type: integer or string + if [[ "$value" =~ ^[0-9]+$ ]]; then + json+=$(printf '\n "%s": %s' "$key" "$value") + else + # Escape backslashes and double quotes for JSON string safety + value="${value//\\/\\\\}" + value="${value//\"/\\\"}" + json+=$(printf '\n "%s": "%s"' "$key" "$value") + fi + done + + json+=$'\n}\n' + + # Create parent directory if needed + local dir + dir=$(dirname "$file") + if [[ ! -d "$dir" ]]; then + mkdir -p "$dir" + fi + + printf '%s' "$json" > "$file" + info "Metadata saved to $file" +} + +# --------------------------------------------------------------------------- +# Main dispatcher +# --------------------------------------------------------------------------- + +if [[ $# -eq 0 ]]; then + usage +fi + +subcommand="$1" +shift + +case "$subcommand" in + preflight) cmd_preflight "$@" ;; + push) cmd_push "$@" ;; + check-existing) cmd_check_existing "$@" ;; + create-pr) cmd_create_pr "$@" ;; + create-mr) cmd_create_mr "$@" ;; + save-metadata) cmd_save_metadata "$@" ;; + -h|--help|help) usage ;; + *) fail "Unknown subcommand: $subcommand. Run with --help for usage." 1 ;; +esac diff --git a/bugfix/SKILL.md b/bugfix/SKILL.md index 20ea747d..5a86db93 100644 --- a/bugfix/SKILL.md +++ b/bugfix/SKILL.md @@ -1,6 +1,6 @@ --- name: bugfix -version: 0.8.0 +version: 0.8.1 description: >- Diagnostic and repair workflow that analyzes error logs, traces root causes, implements fixes, and verifies with regression tests. diff --git a/bugfix/skills/pr.md b/bugfix/skills/pr.md index 88981529..8dbfc38f 100644 --- a/bugfix/skills/pr.md +++ b/bugfix/skills/pr.md @@ -35,6 +35,18 @@ the documented recovery paths instead of guessing. - **Never attempt `gh repo fork` without asking the user first.** - **Never fall back to patch files without exhausting all other options.** +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.sh +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process ### Placeholders Used in This Skill @@ -72,31 +84,26 @@ If the user provides a path or the repo is obvious from session context Run ALL of these before doing anything else. Do not skip any. -**1a. Check GitHub CLI authentication and determine GH_USER:** +**1a. Run the shared pre-flight checks:** ```bash -gh auth status +../../_shared/scripts/publish.sh preflight --platform github ``` -- If authenticated, determine `GH_USER` — the **real user's** GitHub username - (not the bot). Try these in order: +Parse the structured output: +- `auth_ok` — whether `gh auth` succeeded +- `auth_user` — the GitHub username (`GH_USER`) +- `branch` — current branch name +- `has_uncommitted` / `has_staged` — whether there are uncommitted changes -```bash -# Works for normal user tokens: -gh api user --jq .login 2>/dev/null - -# If that fails (403), you're running as a GitHub App/bot. -# Get the real user from the app installation: -gh api /installation/repositories --jq '.repositories[0].owner.login' -``` +If `auth_ok=true`, set `GH_USER` from `auth_user`. If `auth_user` is +empty (GitHub App/bot), the script already tried the +`/installation/repositories` fallback. -The `/installation/repositories` endpoint works because GitHub Apps are -installed on user accounts — the repo owner is the actual user. - -- If not authenticated: note this — several later steps depend on `gh`. But - do NOT dump all manual instructions yet. Continue the remaining pre-flight - checks (1b–1e) to gather as much information as possible from git alone. - After pre-flight, you will present options to the user. +If `auth_ok=false`: note this — several later steps depend on `gh`. But +do NOT dump all manual instructions yet. Continue the remaining pre-flight +checks (1b–1d) to gather as much information as possible from git alone. +After pre-flight, you will present options to the user. **1b. Check git configuration:** @@ -148,15 +155,9 @@ git remote get-url origin | sed -E 's#.*/([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_OWNER/REPO` — you'll need it later. -**1e. Check current branch and changes:** - -```bash -git status -git diff --stat -``` - -Confirm there are actual changes to commit. If there are no changes, stop -and tell the user. +Confirm there are actual changes to commit (from the pre-flight output's +`has_uncommitted` field, or run `git diff --stat`). If there are no +changes, stop and tell the user. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_OWNER/REPO`, which remotes exist, and whether there are changes to @@ -440,10 +441,10 @@ to write an accurate commit message. Don't make up details. ### Step 8: Push to Fork ```bash -git push -u fork bugfix/BRANCH_NAME +../../_shared/scripts/publish.sh push --remote fork --branch bugfix/BRANCH_NAME ``` -**If this fails:** +**If the script exits with code 3 (push failed):** - **Authentication error**: Check `gh auth status` again. The user may need to re-authenticate or the sandbox may be blocking network access. @@ -456,78 +457,57 @@ access. Please run: `git push -u fork BRANCH_NAME`" ### Step 9: Create the Draft PR -**If a pull request already exists** for this branch on -`UPSTREAM_OWNER/REPO`, skip this step and proceed to **Confirm and -Report**. Check with: +**Check for an existing PR** before attempting creation: ```bash -gh pr list --repo UPSTREAM_OWNER/REPO --head bugfix/BRANCH_NAME --json number,url --jq '.[0] // empty' +../../_shared/scripts/publish.sh check-existing \ + --repo UPSTREAM_OWNER/REPO \ + --head bugfix/BRANCH_NAME ``` -If the command fails (auth error, network error, API error), **stop and -report the failure** — do not fall through to PR creation. Only proceed -when the command succeeds: a result means the PR already exists (skip to -Step 10 and report its URL); an empty result means no existing PR (continue -with creation below). +If exit code is 5, a PR already exists — skip to Step 10 and report its +URL. If the command fails (auth error, network error, API error), **stop +and report the failure** — do not fall through to PR creation. **PR title format:** Use **`[ISSUE_KEY]: short description in lowercase`**. If the artifact `.artifacts/bugfix/{issue}/pr-description.md` exists and has a `## Title` line in this format, use that title. Otherwise set `ISSUE_KEY` from the branch name or context (e.g. Jira EDM-1234, GitHub #47) and build the title as `[ISSUE_KEY]: short description`. -**Try `gh pr create` first** (it works for normal user tokens): +**Create the PR using the shared script** (works for normal user tokens): + +If the `--body-file` artifact exists: ```bash -gh pr create \ - --draft \ +../../_shared/scripts/publish.sh create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ --title "[ISSUE_KEY]: short description in lowercase" \ - --body-file .artifacts/bugfix/{issue}/pr-description.md + --body-file .artifacts/bugfix/{issue}/pr-description.md \ + --draft ``` -**Key flags explained:** - -- `--repo`: The upstream repository (where the PR goes). REQUIRED for cross-fork PRs. -- `--head`: Must be `FORK_OWNER:BRANCH_NAME` format for fork-based PRs. Without the - owner prefix, GitHub looks for the branch on the upstream repo and fails. -- `--base`: The target branch on upstream (usually `main`). -- `--draft`: Always submit as draft first. -- `--title`: PR title must be `[ISSUE_KEY]: short description`. Prefer the title from the artifact's `## Title` section if present. -- `--body-file`: Use the PR description artifact if `/document` was run. - -**If `--body-file` artifact doesn't exist**, use `--body` with inline content: +If the artifact doesn't exist, generate the PR body inline (AI-dependent — +see the template in this skill's Notes section) and pass it with `--body`: ```bash -gh pr create \ - --draft \ +../../_shared/scripts/publish.sh create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ --title "[ISSUE_KEY]: short description in lowercase" \ - --body "## Problem -WHAT_WAS_BROKEN - -## Root Cause -WHY_IT_WAS_BROKEN - -## Fix -WHAT_THIS_PR_CHANGES - -## Testing -HOW_THE_FIX_WAS_VERIFIED - -## Confidence -HIGH_MEDIUM_LOW — BRIEF_JUSTIFICATION - -## Rollback -HOW_TO_REVERT_IF_SOMETHING_GOES_WRONG + --body "PR_BODY_TEXT" \ + --draft +``` -## Risk Assessment -LOW_MEDIUM_HIGH — WHAT_COULD_BE_AFFECTED +**Key flags explained:** -Fixes #ISSUE_NUMBER" -``` +- `--repo`: The upstream repository (where the PR goes). REQUIRED for cross-fork PRs. +- `--head`: Must be `FORK_OWNER:BRANCH_NAME` format for fork-based PRs. Without the + owner prefix, GitHub looks for the branch on the upstream repo and fails. +- `--base`: The target branch on upstream (usually `main`). +- `--draft`: Always submit as draft first. +- `--title`: PR title must be `[ISSUE_KEY]: short description`. Prefer the title from the artifact's `## Title` section if present. -**If `gh pr create` fails (403, "Resource not accessible by integration", etc.):** +**If the script exits with code 4 (PR creation failed, e.g., 403, "Resource not accessible by integration"):** This is the expected outcome when running as a GitHub App bot. Do NOT retry, do NOT debug further, do NOT fall back to a patch file. Instead: diff --git a/design/SKILL.md b/design/SKILL.md index fdc4968f..cf183255 100644 --- a/design/SKILL.md +++ b/design/SKILL.md @@ -1,6 +1,6 @@ --- name: design -version: 0.9.1 +version: 0.9.2 description: >- Design-and-decompose workflow that takes a PRD, researches the problem space, drafts a technical design document with a requirement-anchored testplan, diff --git a/design/skills/publish.md b/design/skills/publish.md index fa50aa1b..a30a391c 100644 --- a/design/skills/publish.md +++ b/design/skills/publish.md @@ -21,6 +21,18 @@ the user before taking action. - **No force-push.** No destructive git operations. - **No direct commits to main.** Always use a feature branch. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.sh +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process ### Step 1: Read the Design Document @@ -56,13 +68,13 @@ validated `docs_repo_path` and `docs_repo_remote`. ### Step 3: Pre-Flight Checks -Verify the environment: +Run the shared pre-flight checks from the docs repo directory: ```bash -gh auth status +(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh preflight --platform github) ``` -In the docs repo directory: +Parse the output to confirm `auth_ok=true`. Also verify the docs repo state: ```bash git -C "{docs_repo_path}" remote -v @@ -206,12 +218,14 @@ git -C "{docs_repo_path}" commit -m "Add design document and testplan for {issue ### Step 5: Push and Create PR +Push the branch using the shared script (run from the docs repo): + ```bash -git -C "{docs_repo_path}" push -u origin {branch-name} +(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh push --remote origin --branch {branch-name}) ``` Read the design document and identify specific areas that warrant reviewer -attention: +attention (AI-dependent): - Open questions from Section 9 (list each by title) - Sections with remaining TBD markers - Key architectural decisions that have significant trade-offs @@ -250,36 +264,42 @@ key: if yes, use `{issue-key}: Design - {title}`; otherwise use `Design: {title}`. ```bash -gh pr create --draft --repo {owner}/{repo} --base {base-branch} --head {branch-name} --title "{pr-title}" --body-file .artifacts/design/{issue-key}/08-pr-description.md +../../_shared/scripts/publish.sh create-pr \ + --repo {owner}/{repo} \ + --base {base-branch} \ + --head {branch-name} \ + --title "{pr-title}" \ + --body-file .artifacts/design/{issue-key}/08-pr-description.md \ + --draft ``` -### Step 6: Save Publish Metadata +The script prints the PR URL on stdout. Parse the PR number from the URL path. -Write `.artifacts/design/{issue-key}/publish-metadata.json`: +### Step 6: Save Publish Metadata If `04-testplan.md` was published: -```json -{ - "release": "{release}", - "feature": "{feature}", - "design_file_path": "{release}/{feature}/design.md", - "testplan_file_path": "{release}/{feature}/testplan.md", - "pr_number": {pr-number}, - "branch": "{branch-name}" -} +```bash +../../_shared/scripts/publish.sh save-metadata \ + --file .artifacts/design/{issue-key}/publish-metadata.json \ + release={release} \ + feature={feature} \ + design_file_path={release}/{feature}/design.md \ + testplan_file_path={release}/{feature}/testplan.md \ + pr_number={pr-number} \ + branch={branch-name} ``` -If no testplan was published, omit `testplan_file_path` entirely: +If no testplan was published, omit `testplan_file_path`: -```json -{ - "release": "{release}", - "feature": "{feature}", - "design_file_path": "{release}/{feature}/design.md", - "pr_number": {pr-number}, - "branch": "{branch-name}" -} +```bash +../../_shared/scripts/publish.sh save-metadata \ + --file .artifacts/design/{issue-key}/publish-metadata.json \ + release={release} \ + feature={feature} \ + design_file_path={release}/{feature}/design.md \ + pr_number={pr-number} \ + branch={branch-name} ``` ### Step 7: Report to User diff --git a/docs-writer/SKILL.md b/docs-writer/SKILL.md index b4ca9873..de25ef2a 100644 --- a/docs-writer/SKILL.md +++ b/docs-writer/SKILL.md @@ -1,6 +1,6 @@ --- name: docs-writer -version: 0.3.1 +version: 0.3.2 description: Documentation workflow that converts requirements into structured AsciiDoc sections, runs Vale for style compliance, and produces merge-ready content. Use when creating or updating AsciiDoc documentation from Jira tickets, GitHub issues, or feature descriptions. --- # Docs Writer Workflow Orchestrator diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index dce6262e..0c18b570 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -29,6 +29,20 @@ recovery paths instead of guessing. - **Always create a draft MR.** Let the author mark it ready after review. - **Never attempt `glab repo fork` without asking the user first.** +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.sh +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-mr`, and `save-metadata`. For GitLab workflows, pass +`--platform gitlab` to `preflight` and `check-existing`. See the script +header for full usage. + ## Process ### Placeholders Used in This Skill @@ -47,20 +61,23 @@ These are determined during pre-flight checks. Record each value as you go. Run ALL of these before doing anything else. Do not skip any. -**1a. Check GitLab CLI authentication and determine GL_USER:** +**1a. Run the shared pre-flight checks:** ```bash -glab auth status +../../_shared/scripts/publish.sh preflight --platform gitlab ``` -- If authenticated, determine `GL_USER`: +Parse the structured output: +- `auth_ok` — whether `glab auth` succeeded +- `auth_user` — the GitLab username (`GL_USER`) +- `branch` — current branch name +- `has_uncommitted` / `has_staged` — whether there are uncommitted changes -```bash -glab api user --jq .username -``` +If `auth_ok=true`, set `GL_USER` from `auth_user`. -- If not authenticated: note this and continue the remaining pre-flight checks (1b–1e) to gather as much information as possible from git alone. After pre-flight, present options - to the user. +If `auth_ok=false`: note this and continue the remaining pre-flight checks +(1b–1d) to gather as much information as possible from git alone. After +pre-flight, present options to the user. **1b. Check git configuration:** @@ -111,14 +128,9 @@ git remote get-url origin | sed -E 's#.*[:/]([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_PROJECT`. -**1e. Check current branch and changes:** - -```bash -git status -git diff --stat -``` - -Confirm there are actual changes to commit. If there are no changes, stop and tell the user. +Confirm there are actual changes to commit (from the pre-flight output's +`has_uncommitted` field, or run `git diff --stat`). If there are no +changes, stop and tell the user. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_PROJECT`, which remotes exist, and whether there are changes to commit. You may also know `GL_USER` (if auth is available). @@ -237,16 +249,16 @@ Don't make up details. **Direct push (write access):** ```bash -git push -u origin docs/BRANCH_NAME +../../_shared/scripts/publish.sh push --remote origin --branch docs/BRANCH_NAME ``` **Fork push:** ```bash -git push -u fork docs/BRANCH_NAME +../../_shared/scripts/publish.sh push --remote fork --branch docs/BRANCH_NAME ``` -**If push fails:** +**If the script exits with code 3 (push failed):** - **Authentication error**: Check `glab auth status`. User may need to re-authenticate. - **Permission denied**: Verify the remote URL points to the correct project. @@ -256,36 +268,38 @@ git push -u fork docs/BRANCH_NAME **MR title format:** Use `[TICKET_ID]: short description in lowercase`. +**Building the description:** Use the MR description prepared by the `/apply` +phase at `.artifacts/${ticket_id}/04-mr-description.md`. If the file does not +exist, build the description (AI-dependent) from the context artifact +(`01-context.md`) and plan artifact (`02-plan.md`). + **Direct push (user has write access):** ```bash -glab mr create \ - --draft \ - --source-branch docs/BRANCH_NAME \ - --target-branch main \ +../../_shared/scripts/publish.sh create-mr \ + --source docs/BRANCH_NAME \ + --target main \ --title "[TICKET_ID]: short description" \ - --description "DESCRIPTION" \ - --yes + --desc-file .artifacts/${ticket_id}/04-mr-description.md \ + --draft ``` +If no description file exists, use `--description` with inline text instead. + **Fork workflow:** ```bash -glab mr create \ - --draft \ - --repo UPSTREAM_PROJECT \ +../../_shared/scripts/publish.sh create-mr \ + --project UPSTREAM_PROJECT \ --head FORK_PROJECT \ - --source-branch docs/BRANCH_NAME \ - --target-branch main \ + --source docs/BRANCH_NAME \ + --target main \ --title "[TICKET_ID]: short description" \ - --description "DESCRIPTION" \ - --yes + --desc-file .artifacts/${ticket_id}/04-mr-description.md \ + --draft ``` -**Building the description:** Use the MR description prepared by the `/apply` phase at `.artifacts/${ticket_id}/04-mr-description.md`. If the file does not exist, build the -description from the context artifact (`01-context.md`) and plan artifact (`02-plan.md`). - -**If `glab mr create` fails:** +**If the script exits with code 4 (MR creation failed):** 1. **Write the MR description** to `.artifacts/${ticket_id}/04-mr-description.md` diff --git a/e2e/SKILL.md b/e2e/SKILL.md index 3fd17b10..6003deda 100644 --- a/e2e/SKILL.md +++ b/e2e/SKILL.md @@ -1,6 +1,6 @@ --- name: e2e -version: 0.7.0 +version: 0.7.1 description: >- Story-to-e2e-test workflow that takes a Jira [QE] Story, discovers the project's e2e testing infrastructure, plans test scenarios, writes e2e diff --git a/e2e/skills/publish.md b/e2e/skills/publish.md index 976501e0..a142084d 100644 --- a/e2e/skills/publish.md +++ b/e2e/skills/publish.md @@ -23,6 +23,18 @@ user before taking action. - **No direct commits to main.** The feature branch must already exist from `/code`. - **Validation must have passed.** Check for a passing validation report before proceeding. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.sh +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process ### Step 1: Pre-Flight Checks @@ -48,19 +60,15 @@ Verify readiness: If there are no commits ahead of the Local Base, there's nothing to publish. -3. Check for uncommitted changes: +3. Run the shared pre-flight checks: ```bash - git status + ../../_shared/scripts/publish.sh preflight --platform github ``` - If there are uncommitted changes, ask the user how to proceed. - -4. Verify GitHub CLI is authenticated: - - ```bash - gh auth status - ``` + Parse the output to confirm `auth_ok=true` and check for + `has_uncommitted=true` or `has_staged=true`. If there are uncommitted + changes, ask the user how to proceed. ### Step 2: Cross-Cutting Review @@ -115,7 +123,7 @@ Confirm with the user before proceeding. ### Step 4: Push Branch ```bash -git push -u origin {branch-name} +../../_shared/scripts/publish.sh push --remote origin --branch {branch-name} ``` ### Step 5: Create PR Description @@ -164,11 +172,26 @@ In either case, save the result to Check the **Repository Topology** section of `01-context.md` to determine whether this is a fork-based workflow. +First, check whether a PR already exists for this branch: + +```bash +../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {branch-name} +``` + +If exit code is 5, a PR already exists — skip to Step 7 and use the +returned URL. If exit code is 0, create a new PR. + **If the repo is a fork** (Origin is `{fork-owner}/{repo}`, Upstream is `{upstream-owner}/{repo}`): ```bash -gh pr create --draft --repo {upstream-owner}/{repo} --base {pr-target} --head {fork-owner}:{branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/e2e/{issue-key}/06-pr-description.md +../../_shared/scripts/publish.sh create-pr \ + --repo {upstream-owner}/{repo} \ + --base {pr-target} \ + --head {fork-owner}:{branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/e2e/{issue-key}/06-pr-description.md \ + --draft ``` The `--repo` flag targets the upstream repository (where the PR lives), @@ -178,12 +201,20 @@ branch (on the fork). **If the repo is a direct clone** (not a fork): ```bash -gh pr create --draft --base {pr-target} --head {branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/e2e/{issue-key}/06-pr-description.md +../../_shared/scripts/publish.sh create-pr \ + --base {pr-target} \ + --head {branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/e2e/{issue-key}/06-pr-description.md \ + --draft ``` -Parse the PR number and URL from the `gh pr create` output. The command -prints a URL like `https://github.com/owner/repo/pull/42` — extract the -number from the URL path. +The script prints the PR URL on stdout. Parse the PR number from the URL +path (e.g., `https://github.com/owner/repo/pull/42` → `42`). + +If the script exits with code 4 (PR creation failed), fall back to +providing the user with a GitHub compare URL: +`https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` ### Step 7: Save Publish Metadata @@ -191,37 +222,35 @@ Read `{owner}/{repo}` from the **Origin** field of the Repository Topology section of `01-context.md`. If the repo is a fork, also read the **Upstream** field. -Write `.artifacts/e2e/{issue-key}/publish-metadata.json`. - The `repo` field always refers to where the PR lives. The `origin` field records the repo that was pushed to. **If the repo is a fork** (set `repo` to the upstream, `origin` to the fork): -```json -{ - "repo": "{upstream-owner}/{repo}", - "origin": "{fork-owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +../../_shared/scripts/publish.sh save-metadata \ + --file .artifacts/e2e/{issue-key}/publish-metadata.json \ + repo={upstream-owner}/{repo} \ + origin={fork-owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` **If the repo is a direct clone** (`repo` and `origin` are the same): -```json -{ - "repo": "{owner}/{repo}", - "origin": "{owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +../../_shared/scripts/publish.sh save-metadata \ + --file .artifacts/e2e/{issue-key}/publish-metadata.json \ + repo={owner}/{repo} \ + origin={owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` ### Step 8: Report to User diff --git a/implement/SKILL.md b/implement/SKILL.md index b9fab8e8..232e590b 100644 --- a/implement/SKILL.md +++ b/implement/SKILL.md @@ -1,6 +1,6 @@ --- name: implement -version: 0.9.0 +version: 0.9.1 description: >- Story-to-code workflow that takes a Jira Story, plans the implementation, writes contract-based tests and production code via TDD, validates against diff --git a/implement/skills/publish.md b/implement/skills/publish.md index b58d3e2b..faa87eaa 100644 --- a/implement/skills/publish.md +++ b/implement/skills/publish.md @@ -23,6 +23,18 @@ user before taking action. - **No direct commits to main.** The feature branch must already exist from `/code`. - **Validation must have passed.** Check for a passing validation report before proceeding. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.sh +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process ### Step 1: Pre-Flight Checks @@ -48,19 +60,15 @@ Verify readiness: If there are no commits ahead of the Local Base, there's nothing to publish. -3. Check for uncommitted changes: +3. Run the shared pre-flight checks: ```bash - git status + ../../_shared/scripts/publish.sh preflight --platform github ``` - If there are uncommitted changes, ask the user how to proceed. - -4. Verify GitHub CLI is authenticated: - - ```bash - gh auth status - ``` + Parse the output to confirm `auth_ok=true` and check for + `has_uncommitted=true` or `has_staged=true`. If there are uncommitted + changes, ask the user how to proceed. ### Step 2: Cross-Cutting Review @@ -114,7 +122,7 @@ Confirm with the user before proceeding. ### Step 4: Push Branch ```bash -git push -u origin {branch-name} +../../_shared/scripts/publish.sh push --remote origin --branch {branch-name} ``` ### Step 5: Create PR Description @@ -160,11 +168,26 @@ In either case, save the result to Check the **Repository Topology** section of `01-context.md` to determine whether this is a fork-based workflow. +First, check whether a PR already exists for this branch: + +```bash +../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {branch-name} +``` + +If exit code is 5, a PR already exists — skip to Step 7 and use the +returned URL. If exit code is 0, create a new PR. + **If the repo is a fork** (Origin is `{fork-owner}/{repo}`, Upstream is `{upstream-owner}/{repo}`): ```bash -gh pr create --draft --repo {upstream-owner}/{repo} --base {pr-target} --head {fork-owner}:{branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/implement/{issue-key}/06-pr-description.md +../../_shared/scripts/publish.sh create-pr \ + --repo {upstream-owner}/{repo} \ + --base {pr-target} \ + --head {fork-owner}:{branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/implement/{issue-key}/06-pr-description.md \ + --draft ``` The `--repo` flag targets the upstream repository (where the PR lives), @@ -174,12 +197,20 @@ branch (on the fork). **If the repo is a direct clone** (not a fork): ```bash -gh pr create --draft --base {pr-target} --head {branch-name} --title "{issue-key}: {story title}" --body-file .artifacts/implement/{issue-key}/06-pr-description.md +../../_shared/scripts/publish.sh create-pr \ + --base {pr-target} \ + --head {branch-name} \ + --title "{issue-key}: {story title}" \ + --body-file .artifacts/implement/{issue-key}/06-pr-description.md \ + --draft ``` -Parse the PR number and URL from the `gh pr create` output. The command -prints a URL like `https://github.com/owner/repo/pull/42` — extract the -number from the URL path. +The script prints the PR URL on stdout. Parse the PR number from the URL +path (e.g., `https://github.com/owner/repo/pull/42` → `42`). + +If the script exits with code 4 (PR creation failed), fall back to +providing the user with a GitHub compare URL: +`https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` ### Step 7: Save Publish Metadata @@ -187,37 +218,35 @@ Read `{owner}/{repo}` from the **Origin** field of the Repository Topology section of `01-context.md`. If the repo is a fork, also read the **Upstream** field. -Write `.artifacts/implement/{issue-key}/publish-metadata.json`. - The `repo` field always refers to where the PR lives. The `origin` field records the repo that was pushed to. **If the repo is a fork** (set `repo` to the upstream, `origin` to the fork): -```json -{ - "repo": "{upstream-owner}/{repo}", - "origin": "{fork-owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +../../_shared/scripts/publish.sh save-metadata \ + --file .artifacts/implement/{issue-key}/publish-metadata.json \ + repo={upstream-owner}/{repo} \ + origin={fork-owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` **If the repo is a direct clone** (`repo` and `origin` are the same): -```json -{ - "repo": "{owner}/{repo}", - "origin": "{owner}/{repo}", - "branch": "{branch-name}", - "base": "{pr-target}", - "pr_number": {pr-number}, - "pr_url": "{url from gh pr create output}", - "jira_key": "{issue-key}" -} +```bash +../../_shared/scripts/publish.sh save-metadata \ + --file .artifacts/implement/{issue-key}/publish-metadata.json \ + repo={owner}/{repo} \ + origin={owner}/{repo} \ + branch={branch-name} \ + base={pr-target} \ + pr_number={pr-number} \ + pr_url={url-from-create-pr-output} \ + jira_key={issue-key} ``` ### Step 8: Report to User diff --git a/prd/SKILL.md b/prd/SKILL.md index 4e23ff38..cc2e7677 100644 --- a/prd/SKILL.md +++ b/prd/SKILL.md @@ -1,6 +1,6 @@ --- name: prd -version: 0.9.1 +version: 0.9.2 description: >- Requirements-to-PRD workflow that ingests requirements from Jira, clarifies ambiguities through iterative Q&A, drafts a Product Requirements Document, diff --git a/prd/skills/publish.md b/prd/skills/publish.md index 12237482..dcd5bc06 100644 --- a/prd/skills/publish.md +++ b/prd/skills/publish.md @@ -21,6 +21,18 @@ the user before taking action. - **No force-push.** No destructive git operations. - **No direct commits to main.** Always use a feature branch. +## Shared Script + +This skill delegates deterministic git and CLI operations to a shared +script. Reference it using a relative path from this file: + +``` +../../_shared/scripts/publish.sh +``` + +The script provides subcommands: `preflight`, `push`, `check-existing`, +`create-pr`, and `save-metadata`. See the script header for full usage. + ## Process ### Step 1: Read the PRD @@ -56,13 +68,13 @@ validated `docs_repo_path` and `docs_repo_remote`. ### Step 3: Pre-Flight Checks -Verify the environment using the docs repo: +Run the shared pre-flight checks from the docs repo directory: ```bash -gh auth status +(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh preflight --platform github) ``` -In the docs repo directory: +Parse the output to confirm `auth_ok=true`. Also verify the docs repo state: ```bash git -C "{docs_repo_path}" remote -v @@ -175,12 +187,15 @@ git -C "{docs_repo_path}" commit -m "Add PRD for {issue-key}: {title}" ### Step 5: Push and Create PR +Push the branch using the shared script (run from the docs repo): + ```bash -git -C "{docs_repo_path}" push -u origin {branch-name} +(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh push --remote origin --branch {branch-name}) ``` -Prepare the PR description and save it to `.artifacts/prd/{issue-key}/04-pr-description.md` -(in the source repo's artifact directory): +Prepare the PR description (AI-dependent — summarize the PRD content) and +save it to `.artifacts/prd/{issue-key}/04-pr-description.md` (in the source +repo's artifact directory): ```markdown ## PRD: {title} @@ -208,22 +223,29 @@ create the draft PR. If `{issue-key}` is a Jira key, prefix the title with it (`{issue-key}: PRD - {title}`); otherwise use `PRD: {title}`. ```bash -gh pr create --draft --repo {owner}/{repo} --base {base-branch} --head {branch-name} --title "{issue-key}: PRD - {title}" --body-file .artifacts/prd/{issue-key}/04-pr-description.md +../../_shared/scripts/publish.sh create-pr \ + --repo {owner}/{repo} \ + --base {base-branch} \ + --head {branch-name} \ + --title "{issue-key}: PRD - {title}" \ + --body-file .artifacts/prd/{issue-key}/04-pr-description.md \ + --draft ``` +The script prints the PR URL on stdout. Parse the PR number from the URL path. + ### Step 6: Save Publish Metadata -Write `.artifacts/prd/{issue-key}/publish-metadata.json` to record the -file path and PR details for use by `/revise` and `/respond`: - -```json -{ - "release": "{release}", - "feature": "{feature}", - "prd_file_path": "{release}/{feature}/prd.md", - "pr_number": {pr-number}, - "branch": "{branch-name}" -} +Save metadata for use by `/revise` and `/respond`: + +```bash +../../_shared/scripts/publish.sh save-metadata \ + --file .artifacts/prd/{issue-key}/publish-metadata.json \ + release={release} \ + feature={feature} \ + prd_file_path={release}/{feature}/prd.md \ + pr_number={pr-number} \ + branch={branch-name} ``` ### Step 7: Report to User From f3321466d14b953d907ec3fb9addabbe012bfb23 Mon Sep 17 00:00:00 2001 From: Chai Bot <chai-bot@redhat.com> Date: Wed, 9 Sep 2026 17:43:54 +0000 Subject: [PATCH 2/6] Fix review feedback on publish script and skill files Address all 14 CodeRabbit review comments on PR #116: Script fixes (_shared/scripts/publish.sh): - Fix glab mr list to use --output json instead of --json (Fix 1) - Remove || true from check-existing; handle exit codes explicitly (Fix 2) - Redirect stderr to temp file instead of merging with stdout in create-pr and create-mr (Fix 3) - Add flag_value() helper to prevent cryptic unbound variable errors when a flag is passed without its value under set -u (Fix 4) - Update exit code docs: remove unused exit code 2, clarify preflight reports via structured output (Fix 8) - Always serialize save-metadata values as JSON strings to avoid leading-zero truncation (Fix 9) Skill file fixes: - Check both has_uncommitted and has_staged in bugfix/skills/pr.md and docs-writer/skills/create-mr.md (Fix 5) - Use discovered remote name placeholder instead of hardcoded origin/fork in docs-writer/skills/create-mr.md (Fix 6) - Resolve publish script to absolute path before cd in design and prd publish skills (Fix 7) - Add check-existing step before create-pr in design/skills/publish.md (Fix 10) - Carry pr_number from check-existing into metadata in e2e and implement publish skills (Fix 11) - Stop on auth_ok=false in prd/skills/publish.md (Fix 12) - Document correct compare URL format for direct clones vs forks in e2e/skills/publish.md (Fix 13) - Use owner:branch format for fork-based check-existing in e2e/skills/publish.md (Fix 14) --- _shared/scripts/publish.sh | 107 +++++++++++++++++++------------- bugfix/skills/pr.md | 4 +- design/skills/publish.md | 33 ++++++++-- docs-writer/skills/create-mr.md | 17 +++-- e2e/skills/publish.md | 18 +++++- implement/skills/publish.md | 6 +- prd/skills/publish.md | 27 ++++++-- 7 files changed, 142 insertions(+), 70 deletions(-) diff --git a/_shared/scripts/publish.sh b/_shared/scripts/publish.sh index 9584215f..7e88b097 100755 --- a/_shared/scripts/publish.sh +++ b/_shared/scripts/publish.sh @@ -26,9 +26,8 @@ # publish.sh save-metadata --file <path> [key=value ...] # # Exit codes: -# 0 — success +# 0 — success (preflight reports issues via structured output, not exit codes) # 1 — missing argument or configuration error -# 2 — pre-flight check failed (auth, branch, or changes issue) # 3 — push failed # 4 — PR/MR creation failed # 5 — existing PR/MR found (check-existing only; prints details on stdout) @@ -59,6 +58,16 @@ require_arg() { fi } +flag_value() { + # Ensure a flag has an accompanying value argument. Prevents cryptic + # "unbound variable" errors under set -u when a flag is passed without + # its value (e.g., --platform with no argument). + # Usage (inside a while/case loop): flag_value "--flag-name" "$#" + if [[ $2 -lt 2 ]]; then + fail "$1 requires a value" 1 + fi +} + # --------------------------------------------------------------------------- # Subcommand: preflight # --------------------------------------------------------------------------- @@ -74,7 +83,7 @@ cmd_preflight() { while [[ $# -gt 0 ]]; do case "$1" in - --platform) platform="$2"; shift 2 ;; + --platform) flag_value "$1" "$#"; platform="$2"; shift 2 ;; *) fail "preflight: unknown flag: $1" 1 ;; esac done @@ -144,8 +153,8 @@ cmd_push() { while [[ $# -gt 0 ]]; do case "$1" in - --remote) remote="$2"; shift 2 ;; - --branch) branch="$2"; shift 2 ;; + --remote) flag_value "$1" "$#"; remote="$2"; shift 2 ;; + --branch) flag_value "$1" "$#"; branch="$2"; shift 2 ;; *) fail "push: unknown flag: $1" 1 ;; esac done @@ -187,9 +196,9 @@ cmd_check_existing() { while [[ $# -gt 0 ]]; do case "$1" in - --repo) repo="$2"; shift 2 ;; - --head) head="$2"; shift 2 ;; - --platform) platform="$2"; shift 2 ;; + --repo) flag_value "$1" "$#"; repo="$2"; shift 2 ;; + --head) flag_value "$1" "$#"; head="$2"; shift 2 ;; + --platform) flag_value "$1" "$#"; platform="$2"; shift 2 ;; *) fail "check-existing: unknown flag: $1" 1 ;; esac done @@ -199,9 +208,12 @@ cmd_check_existing() { case "$platform" in github) - local result + local result exit_code=0 result=$(gh pr list --repo "$repo" --head "$head" \ - --json number,url --jq '.[0] // empty' 2>/dev/null || true) + --json number,url --jq '.[0] // empty' 2>/dev/null) || exit_code=$? + if [[ $exit_code -ne 0 ]]; then + fail "check-existing: GitHub API query failed (exit $exit_code). Check gh auth status." 1 + fi if [[ -n "$result" ]]; then echo "$result" exit 5 @@ -209,9 +221,14 @@ cmd_check_existing() { ;; gitlab) local source_branch="$head" + local raw exit_code=0 + raw=$(glab mr list --repo "$repo" --source-branch "$source_branch" \ + --output json 2>/dev/null) || exit_code=$? + if [[ $exit_code -ne 0 ]]; then + fail "check-existing: GitLab API query failed (exit $exit_code). Check glab auth status." 1 + fi local result - result=$(glab mr list --repo "$repo" --source-branch "$source_branch" \ - --json iid,web_url --jq '.[0] // empty' 2>/dev/null || true) + result=$(printf '%s' "$raw" | jq -r '.[0] // empty' 2>/dev/null) if [[ -n "$result" ]]; then echo "$result" exit 5 @@ -254,15 +271,15 @@ cmd_create_pr() { while [[ $# -gt 0 ]]; do case "$1" in - --repo) repo="$2"; shift 2 ;; - --base) base="$2"; shift 2 ;; - --head) head="$2"; shift 2 ;; - --title) title="$2"; shift 2 ;; - --body-file) body_file="$2"; shift 2 ;; - --body) body="$2"; shift 2 ;; + --repo) flag_value "$1" "$#"; repo="$2"; shift 2 ;; + --base) flag_value "$1" "$#"; base="$2"; shift 2 ;; + --head) flag_value "$1" "$#"; head="$2"; shift 2 ;; + --title) flag_value "$1" "$#"; title="$2"; shift 2 ;; + --body-file) flag_value "$1" "$#"; body_file="$2"; shift 2 ;; + --body) flag_value "$1" "$#"; body="$2"; shift 2 ;; --draft) draft="true"; shift ;; --no-draft) draft="false"; shift ;; - --labels) labels="$2"; shift 2 ;; + --labels) flag_value "$1" "$#"; labels="$2"; shift 2 ;; *) fail "create-pr: unknown flag: $1" 1 ;; esac done @@ -301,14 +318,18 @@ cmd_create_pr() { info "Creating PR: ${title}" local pr_url - if pr_url=$("${cmd[@]}" 2>&1); then + local stderr_file + stderr_file=$(mktemp) + if pr_url=$("${cmd[@]}" 2>"$stderr_file"); then + rm -f "$stderr_file" # gh pr create prints the URL on success echo "$pr_url" info "PR created: $pr_url" else # Print the error but use a distinct exit code so the skill can # detect the failure and fall back (e.g., to a compare URL). - printf '%s\n' "$pr_url" >&2 + cat "$stderr_file" >&2 + rm -f "$stderr_file" exit 4 fi } @@ -343,15 +364,15 @@ cmd_create_mr() { while [[ $# -gt 0 ]]; do case "$1" in - --project) project="$2"; shift 2 ;; - --source) source_branch="$2"; shift 2 ;; - --target) target_branch="$2"; shift 2 ;; - --title) title="$2"; shift 2 ;; - --description) description="$2"; shift 2 ;; - --desc-file) desc_file="$2"; shift 2 ;; + --project) flag_value "$1" "$#"; project="$2"; shift 2 ;; + --source) flag_value "$1" "$#"; source_branch="$2"; shift 2 ;; + --target) flag_value "$1" "$#"; target_branch="$2"; shift 2 ;; + --title) flag_value "$1" "$#"; title="$2"; shift 2 ;; + --description) flag_value "$1" "$#"; description="$2"; shift 2 ;; + --desc-file) flag_value "$1" "$#"; desc_file="$2"; shift 2 ;; --draft) draft="true"; shift ;; --no-draft) draft="false"; shift ;; - --head) head_project="$2"; shift 2 ;; + --head) flag_value "$1" "$#"; head_project="$2"; shift 2 ;; *) fail "create-mr: unknown flag: $1" 1 ;; esac done @@ -391,11 +412,15 @@ cmd_create_mr() { info "Creating MR: ${title}" local mr_url - if mr_url=$("${cmd[@]}" 2>&1); then + local stderr_file + stderr_file=$(mktemp) + if mr_url=$("${cmd[@]}" 2>"$stderr_file"); then + rm -f "$stderr_file" echo "$mr_url" info "MR created: $mr_url" else - printf '%s\n' "$mr_url" >&2 + cat "$stderr_file" >&2 + rm -f "$stderr_file" exit 4 fi } @@ -408,9 +433,9 @@ cmd_create_mr() { # Flags: # --file <path> Output file path (required) # -# Remaining positional arguments are key=value pairs. Values that look -# like integers are stored as JSON numbers; everything else is a JSON -# string. +# Remaining positional arguments are key=value pairs. All values are +# stored as JSON strings to avoid leading-zero truncation and to keep +# the output type-stable. # # Example: # publish.sh save-metadata --file .artifacts/impl/EDM-1/publish-metadata.json \ @@ -423,7 +448,7 @@ cmd_save_metadata() { while [[ $# -gt 0 ]]; do case "$1" in - --file) file="$2"; shift 2 ;; + --file) flag_value "$1" "$#"; file="$2"; shift 2 ;; *=*) pairs+=("$1"); shift ;; *) fail "save-metadata: unexpected argument: $1 (expected key=value)" 1 ;; esac @@ -452,15 +477,11 @@ cmd_save_metadata() { json+="," fi - # Determine JSON type: integer or string - if [[ "$value" =~ ^[0-9]+$ ]]; then - json+=$(printf '\n "%s": %s' "$key" "$value") - else - # Escape backslashes and double quotes for JSON string safety - value="${value//\\/\\\\}" - value="${value//\"/\\\"}" - json+=$(printf '\n "%s": "%s"' "$key" "$value") - fi + # Always serialize as a JSON string to avoid leading-zero truncation + # (e.g., "007" → 7) and to keep the output type-stable. + value="${value//\\/\\\\}" + value="${value//\"/\\\"}" + json+=$(printf '\n "%s": "%s"' "$key" "$value") done json+=$'\n}\n' diff --git a/bugfix/skills/pr.md b/bugfix/skills/pr.md index 8dbfc38f..e4de39dd 100644 --- a/bugfix/skills/pr.md +++ b/bugfix/skills/pr.md @@ -156,8 +156,8 @@ git remote get-url origin | sed -E 's#.*/([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_OWNER/REPO` — you'll need it later. Confirm there are actual changes to commit (from the pre-flight output's -`has_uncommitted` field, or run `git diff --stat`). If there are no -changes, stop and tell the user. +`has_uncommitted` or `has_staged` fields, or run `git diff --stat`). If +both are `false`, there are no changes — stop and tell the user. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_OWNER/REPO`, which remotes exist, and whether there are changes to diff --git a/design/skills/publish.md b/design/skills/publish.md index a30a391c..f51a4816 100644 --- a/design/skills/publish.md +++ b/design/skills/publish.md @@ -35,6 +35,18 @@ The script provides subcommands: `preflight`, `push`, `check-existing`, ## Process +### Step 0: Resolve Script Path + +Before any `cd` or subshell that changes the working directory, resolve +the shared script to an absolute path so it remains valid: + +```bash +PUBLISH_SCRIPT="$(cd "$(dirname "../../_shared/scripts/publish.sh")" && pwd)/publish.sh" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands (Steps 3, 5, 6). + ### Step 1: Read the Design Document Read `.artifacts/design/{issue-key}/03-design.md`. @@ -71,7 +83,7 @@ validated `docs_repo_path` and `docs_repo_remote`. Run the shared pre-flight checks from the docs repo directory: ```bash -(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh preflight --platform github) +(cd "{docs_repo_path}" && "$PUBLISH_SCRIPT" preflight --platform github) ``` Parse the output to confirm `auth_ok=true`. Also verify the docs repo state: @@ -221,7 +233,7 @@ git -C "{docs_repo_path}" commit -m "Add design document and testplan for {issue Push the branch using the shared script (run from the docs repo): ```bash -(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh push --remote origin --branch {branch-name}) +(cd "{docs_repo_path}" && "$PUBLISH_SCRIPT" push --remote origin --branch {branch-name}) ``` Read the design document and identify specific areas that warrant reviewer @@ -263,8 +275,19 @@ draft PR. Set `{pr-title}` based on whether `{issue-key}` is a Jira key: if yes, use `{issue-key}: Design - {title}`; otherwise use `Design: {title}`. +First, check whether a PR already exists for this branch: + +```bash +"$PUBLISH_SCRIPT" check-existing --repo {owner}/{repo} --head {branch-name} +``` + +If exit code is 5, a PR already exists — skip to Step 6 and report its +URL. Parse the PR number from the returned JSON. If the command fails +(non-zero exit other than 5), stop and report the error. If exit code +is 0, create a new PR: + ```bash -../../_shared/scripts/publish.sh create-pr \ +"$PUBLISH_SCRIPT" create-pr \ --repo {owner}/{repo} \ --base {base-branch} \ --head {branch-name} \ @@ -280,7 +303,7 @@ The script prints the PR URL on stdout. Parse the PR number from the URL path. If `04-testplan.md` was published: ```bash -../../_shared/scripts/publish.sh save-metadata \ +"$PUBLISH_SCRIPT" save-metadata \ --file .artifacts/design/{issue-key}/publish-metadata.json \ release={release} \ feature={feature} \ @@ -293,7 +316,7 @@ If `04-testplan.md` was published: If no testplan was published, omit `testplan_file_path`: ```bash -../../_shared/scripts/publish.sh save-metadata \ +"$PUBLISH_SCRIPT" save-metadata \ --file .artifacts/design/{issue-key}/publish-metadata.json \ release={release} \ feature={feature} \ diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index 0c18b570..2fab3595 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -55,6 +55,7 @@ These are determined during pre-flight checks. Record each value as you go. | `UPSTREAM_PROJECT` | Step 1d: project path from remote URL | `red-hat-enterprise-openshift-documentation/edge-manager` | | `FORK_PROJECT` | Step 2: user's fork path | `jsmith/edge-manager` | | `BRANCH_NAME` | Step 4: the branch you create | `docs/RHEM-456-enrollment-api` | +| `PUSH_REMOTE` | Step 2/3: remote name to push to | `origin` or `fork` | | `TICKET_ID` | From artifacts directory or user input | `RHEM-456` | ### Step 1: Pre-flight Checks @@ -129,8 +130,8 @@ git remote get-url origin | sed -E 's#.*[:/]([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_PROJECT`. Confirm there are actual changes to commit (from the pre-flight output's -`has_uncommitted` field, or run `git diff --stat`). If there are no -changes, stop and tell the user. +`has_uncommitted` or `has_staged` fields, or run `git diff --stat`). If +both are `false`, there are no changes — stop and tell the user. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_PROJECT`, which remotes exist, and whether there are changes to commit. You may also know `GL_USER` (if auth is available). @@ -246,16 +247,12 @@ Don't make up details. ### Step 6: Push -**Direct push (write access):** +Use the remote identified during Step 2 (direct push) or Step 3 (fork +workflow) as `PUSH_REMOTE`. Do not hardcode `origin` or `fork` — use the +actual remote name discovered from `git remote -v`. ```bash -../../_shared/scripts/publish.sh push --remote origin --branch docs/BRANCH_NAME -``` - -**Fork push:** - -```bash -../../_shared/scripts/publish.sh push --remote fork --branch docs/BRANCH_NAME +../../_shared/scripts/publish.sh push --remote {push-remote} --branch docs/BRANCH_NAME ``` **If the script exits with code 3 (push failed):** diff --git a/e2e/skills/publish.md b/e2e/skills/publish.md index a142084d..64c38f31 100644 --- a/e2e/skills/publish.md +++ b/e2e/skills/publish.md @@ -174,12 +174,22 @@ whether this is a fork-based workflow. First, check whether a PR already exists for this branch: +For fork-based workflows, use `{fork-owner}:{branch-name}` as the +`--head` value so the check matches only PRs from this fork (plain +`{branch-name}` would match any fork's branch with the same name): + ```bash +# Fork-based: +../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {fork-owner}:{branch-name} + +# Direct clone: ../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {branch-name} ``` -If exit code is 5, a PR already exists — skip to Step 7 and use the -returned URL. If exit code is 0, create a new PR. +If exit code is 5, a PR already exists — parse the PR number and URL +from the returned JSON output, then skip to Step 7 and use those values +in the metadata. If the command fails (non-zero exit other than 5), +stop and report the error. If exit code is 0, create a new PR. **If the repo is a fork** (Origin is `{fork-owner}/{repo}`, Upstream is `{upstream-owner}/{repo}`): @@ -214,7 +224,9 @@ path (e.g., `https://github.com/owner/repo/pull/42` → `42`). If the script exits with code 4 (PR creation failed), fall back to providing the user with a GitHub compare URL: -`https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` + +- Fork-based: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` +- Direct clone: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{branch-name}?expand=1` ### Step 7: Save Publish Metadata diff --git a/implement/skills/publish.md b/implement/skills/publish.md index faa87eaa..89522b84 100644 --- a/implement/skills/publish.md +++ b/implement/skills/publish.md @@ -174,8 +174,10 @@ First, check whether a PR already exists for this branch: ../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {branch-name} ``` -If exit code is 5, a PR already exists — skip to Step 7 and use the -returned URL. If exit code is 0, create a new PR. +If exit code is 5, a PR already exists — parse the PR number and URL +from the returned JSON output, then skip to Step 7 and use those values +in the metadata. If the command fails (non-zero exit other than 5), +stop and report the error. If exit code is 0, create a new PR. **If the repo is a fork** (Origin is `{fork-owner}/{repo}`, Upstream is `{upstream-owner}/{repo}`): diff --git a/prd/skills/publish.md b/prd/skills/publish.md index dcd5bc06..308bc4cb 100644 --- a/prd/skills/publish.md +++ b/prd/skills/publish.md @@ -35,6 +35,18 @@ The script provides subcommands: `preflight`, `push`, `check-existing`, ## Process +### Step 0: Resolve Script Path + +Before any `cd` or subshell that changes the working directory, resolve +the shared script to an absolute path so it remains valid: + +```bash +PUBLISH_SCRIPT="$(cd "$(dirname "../../_shared/scripts/publish.sh")" && pwd)/publish.sh" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands (Steps 3, 5, 6). + ### Step 1: Read the PRD Read `.artifacts/prd/{issue-key}/03-prd.md`. @@ -71,10 +83,15 @@ validated `docs_repo_path` and `docs_repo_remote`. Run the shared pre-flight checks from the docs repo directory: ```bash -(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh preflight --platform github) +(cd "{docs_repo_path}" && "$PUBLISH_SCRIPT" preflight --platform github) ``` -Parse the output to confirm `auth_ok=true`. Also verify the docs repo state: +Parse the output and check `auth_ok`. If `auth_ok=false`, **stop and +tell the user** that GitHub CLI authentication is required to push and +create a PR. Suggest running `gh auth login` and retrying `/publish`. +Do not continue to later steps without authentication. + +If `auth_ok=true`, verify the docs repo state: ```bash git -C "{docs_repo_path}" remote -v @@ -190,7 +207,7 @@ git -C "{docs_repo_path}" commit -m "Add PRD for {issue-key}: {title}" Push the branch using the shared script (run from the docs repo): ```bash -(cd "{docs_repo_path}" && ../../_shared/scripts/publish.sh push --remote origin --branch {branch-name}) +(cd "{docs_repo_path}" && "$PUBLISH_SCRIPT" push --remote origin --branch {branch-name}) ``` Prepare the PR description (AI-dependent — summarize the PRD content) and @@ -223,7 +240,7 @@ create the draft PR. If `{issue-key}` is a Jira key, prefix the title with it (`{issue-key}: PRD - {title}`); otherwise use `PRD: {title}`. ```bash -../../_shared/scripts/publish.sh create-pr \ +"$PUBLISH_SCRIPT" create-pr \ --repo {owner}/{repo} \ --base {base-branch} \ --head {branch-name} \ @@ -239,7 +256,7 @@ The script prints the PR URL on stdout. Parse the PR number from the URL path. Save metadata for use by `/revise` and `/respond`: ```bash -../../_shared/scripts/publish.sh save-metadata \ +"$PUBLISH_SCRIPT" save-metadata \ --file .artifacts/prd/{issue-key}/publish-metadata.json \ release={release} \ feature={feature} \ From d305a84e93a6c53b600059670917f35af6d36832 Mon Sep 17 00:00:00 2001 From: Chai Bot <chai-bot@redhat.com> Date: Wed, 9 Sep 2026 18:14:12 +0000 Subject: [PATCH 3/6] fix: address CodeRabbit review round 2 findings - Add json_escape() with full JSON string escaping for backslash, quotes, newlines, tabs, carriage return, and control chars (#1, #10) - Wire up PUSH_REMOTE variable in docs-writer create-mr (#2, #7) - Add PUBLISH_SCRIPT path resolution via git rev-parse to e2e, bugfix, implement, docs-writer; update design/prd to use same method (#3, #9) - Add docs repo dirty-state check in prd before file copy (#4) - Add check-existing step before create-pr in prd (#5) - Detect untracked files in preflight (has_untracked); update bugfix and docs-writer to warn about untracked changes (#6) - Fix fork-aware check-existing: split owner:branch and filter by headRepositoryOwner instead of passing to --head directly (#8) - Add function-level docstrings to all helpers and subcommands (#13) - Rename Step 0 to Prerequisites in design/prd for valid numbering (#14) - Add fork vs direct-clone compare URL conditional in implement (#12) Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> --- _shared/scripts/publish.sh | 86 +++++++++++++++++++++++++++------ bugfix/skills/pr.md | 30 +++++++++--- design/skills/publish.md | 6 +-- docs-writer/skills/create-mr.md | 36 ++++++++++---- e2e/skills/publish.md | 28 ++++++++--- implement/skills/publish.md | 30 +++++++++--- prd/skills/publish.md | 33 +++++++++++-- 7 files changed, 192 insertions(+), 57 deletions(-) diff --git a/_shared/scripts/publish.sh b/_shared/scripts/publish.sh index 7e88b097..fcee4ec6 100755 --- a/_shared/scripts/publish.sh +++ b/_shared/scripts/publish.sh @@ -38,41 +38,78 @@ set -euo pipefail # Helpers # --------------------------------------------------------------------------- +# Print an error message to stderr and exit with the given code. fail() { printf 'ERROR: %s\n' "$1" >&2 exit "${2:-1}" } +# Print an informational message to stderr. info() { printf 'INFO: %s\n' "$1" >&2 } +# Print usage information extracted from the script header and exit. usage() { sed -n '/^# Usage:/,/^# Exit codes:/{ /^# Exit codes:/d; s/^# \?//; p }' "$0" >&2 exit 1 } +# Fail with a descriptive message if a required argument is empty. require_arg() { if [[ -z "${2:-}" ]]; then fail "Missing required argument: $1" 1 fi } +# Validate that a CLI flag has an accompanying value argument. flag_value() { - # Ensure a flag has an accompanying value argument. Prevents cryptic - # "unbound variable" errors under set -u when a flag is passed without - # its value (e.g., --platform with no argument). + # Prevents cryptic "unbound variable" errors under set -u when a flag + # is passed without its value (e.g., --platform with no argument). # Usage (inside a while/case loop): flag_value "--flag-name" "$#" if [[ $2 -lt 2 ]]; then fail "$1 requires a value" 1 fi } +# Escape a string for safe embedding as a JSON string value. +# Handles backslash, double-quote, newline, tab, carriage return, and +# other control characters (0x00-0x1F, 0x7F) using \uXXXX notation. +# Does not add surrounding quotes — the caller wraps the result. +json_escape() { + local s="$1" + local out="" + local i char code + for (( i = 0; i < ${#s}; i++ )); do + char="${s:i:1}" + case "$char" in + \\) out+="\\\\" ;; + '"') out+="\\\"" ;; + $'\n') out+="\\n" ;; + $'\t') out+="\\t" ;; + $'\r') out+="\\r" ;; + $'\b') out+="\\b" ;; + $'\x0c') out+="\\f" ;; + *) + # Detect remaining control characters (0x00-0x1F, 0x7F) + printf -v code '%d' "'${char}" + if (( code >= 0 && code < 32 )) || (( code == 127 )); then + printf -v char '\\u%04x' "$code" + out+="${char}" + else + out+="${char}" + fi + ;; + esac + done + printf '%s' "$out" +} + # --------------------------------------------------------------------------- # Subcommand: preflight # --------------------------------------------------------------------------- -# Checks authentication (gh or glab), current branch, and uncommitted -# changes. Prints a structured status block on stdout for the calling +# Run pre-flight checks: auth, branch, and working-tree cleanliness. +# Prints a structured key=value status block on stdout for the calling # skill to parse. # # Flags: @@ -93,6 +130,7 @@ cmd_preflight() { local branch="" local has_uncommitted="false" local has_staged="false" + local has_untracked="false" # -- Auth check -- case "$platform" in @@ -126,6 +164,9 @@ cmd_preflight() { if ! git diff --cached --quiet 2>/dev/null; then has_staged="true" fi + if [[ -n "$(git ls-files --others --exclude-standard 2>/dev/null)" ]]; then + has_untracked="true" + fi # -- Output structured block -- cat <<EOF @@ -134,6 +175,7 @@ auth_user=${auth_user} branch=${branch} has_uncommitted=${has_uncommitted} has_staged=${has_staged} +has_untracked=${has_untracked} platform=${platform} EOF } @@ -141,7 +183,7 @@ EOF # --------------------------------------------------------------------------- # Subcommand: push # --------------------------------------------------------------------------- -# Pushes a branch to the specified remote with -u (set upstream). +# Push a branch to the specified remote with upstream tracking (-u). # # Flags: # --remote <name> Git remote name (e.g., fork, origin) @@ -178,8 +220,8 @@ cmd_push() { # --------------------------------------------------------------------------- # Subcommand: check-existing # --------------------------------------------------------------------------- -# Checks whether a PR (GitHub) or MR (GitLab) already exists for the -# given branch. Prints the PR/MR number and URL on stdout if found. +# Check whether an open PR (GitHub) or MR (GitLab) already exists for +# a branch. Supports owner:branch format for fork-aware matching. # # Flags: # --repo <owner/repo> Target repository @@ -209,8 +251,19 @@ cmd_check_existing() { case "$platform" in github) local result exit_code=0 - result=$(gh pr list --repo "$repo" --head "$head" \ - --json number,url --jq '.[0] // empty' 2>/dev/null) || exit_code=$? + if [[ "$head" == *:* ]]; then + # owner:branch format — gh pr list --head does not support this + # syntax. Search by branch name and filter by head repo owner. + local head_owner="${head%%:*}" + local head_branch="${head#*:}" + result=$(gh pr list --repo "$repo" --head "$head_branch" \ + --json number,url,headRepositoryOwner \ + --jq "[.[] | select(.headRepositoryOwner.login == \"$head_owner\")] | .[0] // empty" \ + 2>/dev/null) || exit_code=$? + else + result=$(gh pr list --repo "$repo" --head "$head" \ + --json number,url --jq '.[0] // empty' 2>/dev/null) || exit_code=$? + fi if [[ $exit_code -ne 0 ]]; then fail "check-existing: GitHub API query failed (exit $exit_code). Check gh auth status." 1 fi @@ -244,7 +297,7 @@ cmd_check_existing() { # --------------------------------------------------------------------------- # Subcommand: create-pr # --------------------------------------------------------------------------- -# Creates a GitHub pull request via gh CLI. +# Create a GitHub pull request via the gh CLI. # # Flags: # --repo <owner/repo> Target repository (required for fork-based PRs) @@ -337,7 +390,7 @@ cmd_create_pr() { # --------------------------------------------------------------------------- # Subcommand: create-mr # --------------------------------------------------------------------------- -# Creates a GitLab merge request via glab CLI. +# Create a GitLab merge request via the glab CLI. # # Flags: # --project <path> Upstream project path (for fork-based MRs) @@ -428,7 +481,7 @@ cmd_create_mr() { # --------------------------------------------------------------------------- # Subcommand: save-metadata # --------------------------------------------------------------------------- -# Writes a JSON metadata file from key=value pairs. +# Write a JSON metadata file from key=value pairs with full escaping. # # Flags: # --file <path> Output file path (required) @@ -478,9 +531,10 @@ cmd_save_metadata() { fi # Always serialize as a JSON string to avoid leading-zero truncation - # (e.g., "007" → 7) and to keep the output type-stable. - value="${value//\\/\\\\}" - value="${value//\"/\\\"}" + # (e.g., "007" → 7) and to keep the output type-stable. Full JSON + # escaping handles newlines, tabs, quotes, backslashes, and control + # characters that would otherwise produce invalid JSON. + value=$(json_escape "$value") json+=$(printf '\n "%s": "%s"' "$key" "$value") done diff --git a/bugfix/skills/pr.md b/bugfix/skills/pr.md index e4de39dd..9d95ca0c 100644 --- a/bugfix/skills/pr.md +++ b/bugfix/skills/pr.md @@ -47,6 +47,18 @@ script. Reference it using a relative path from this file: The script provides subcommands: `preflight`, `push`, `check-existing`, `create-pr`, and `save-metadata`. See the script header for full usage. +### Prerequisites: Resolve Script Path + +Before running any subcommands, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ## Process ### Placeholders Used in This Skill @@ -87,14 +99,14 @@ Run ALL of these before doing anything else. Do not skip any. **1a. Run the shared pre-flight checks:** ```bash -../../_shared/scripts/publish.sh preflight --platform github +$PUBLISH_SCRIPT preflight --platform github ``` Parse the structured output: - `auth_ok` — whether `gh auth` succeeded - `auth_user` — the GitHub username (`GH_USER`) - `branch` — current branch name -- `has_uncommitted` / `has_staged` — whether there are uncommitted changes +- `has_uncommitted` / `has_staged` / `has_untracked` — whether there are uncommitted, staged, or untracked changes If `auth_ok=true`, set `GH_USER` from `auth_user`. If `auth_user` is empty (GitHub App/bot), the script already tried the @@ -156,8 +168,10 @@ git remote get-url origin | sed -E 's#.*/([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_OWNER/REPO` — you'll need it later. Confirm there are actual changes to commit (from the pre-flight output's -`has_uncommitted` or `has_staged` fields, or run `git diff --stat`). If -both are `false`, there are no changes — stop and tell the user. +`has_uncommitted`, `has_staged`, or `has_untracked` fields). If all three +are `false`, there are no changes — stop and tell the user. If +`has_untracked` is `true`, warn the user about untracked files and ask +whether they should be included in the commit. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_OWNER/REPO`, which remotes exist, and whether there are changes to @@ -441,7 +455,7 @@ to write an accurate commit message. Don't make up details. ### Step 8: Push to Fork ```bash -../../_shared/scripts/publish.sh push --remote fork --branch bugfix/BRANCH_NAME +$PUBLISH_SCRIPT push --remote fork --branch bugfix/BRANCH_NAME ``` **If the script exits with code 3 (push failed):** @@ -460,7 +474,7 @@ access. Please run: `git push -u fork BRANCH_NAME`" **Check for an existing PR** before attempting creation: ```bash -../../_shared/scripts/publish.sh check-existing \ +$PUBLISH_SCRIPT check-existing \ --repo UPSTREAM_OWNER/REPO \ --head bugfix/BRANCH_NAME ``` @@ -476,7 +490,7 @@ and report the failure** — do not fall through to PR creation. If the `--body-file` artifact exists: ```bash -../../_shared/scripts/publish.sh create-pr \ +$PUBLISH_SCRIPT create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ @@ -489,7 +503,7 @@ If the artifact doesn't exist, generate the PR body inline (AI-dependent — see the template in this skill's Notes section) and pass it with `--body`: ```bash -../../_shared/scripts/publish.sh create-pr \ +$PUBLISH_SCRIPT create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ diff --git a/design/skills/publish.md b/design/skills/publish.md index f51a4816..116b2ccb 100644 --- a/design/skills/publish.md +++ b/design/skills/publish.md @@ -35,17 +35,17 @@ The script provides subcommands: `preflight`, `push`, `check-existing`, ## Process -### Step 0: Resolve Script Path +### Prerequisites: Resolve Script Path Before any `cd` or subshell that changes the working directory, resolve the shared script to an absolute path so it remains valid: ```bash -PUBLISH_SCRIPT="$(cd "$(dirname "../../_shared/scripts/publish.sh")" && pwd)/publish.sh" +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" ``` Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent -commands (Steps 3, 5, 6). +commands. ### Step 1: Read the Design Document diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index 2fab3595..29425f04 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -43,6 +43,18 @@ The script provides subcommands: `preflight`, `push`, `check-existing`, `--platform gitlab` to `preflight` and `check-existing`. See the script header for full usage. +### Prerequisites: Resolve Script Path + +Before running any subcommands, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ## Process ### Placeholders Used in This Skill @@ -65,14 +77,14 @@ Run ALL of these before doing anything else. Do not skip any. **1a. Run the shared pre-flight checks:** ```bash -../../_shared/scripts/publish.sh preflight --platform gitlab +$PUBLISH_SCRIPT preflight --platform gitlab ``` Parse the structured output: - `auth_ok` — whether `glab auth` succeeded - `auth_user` — the GitLab username (`GL_USER`) - `branch` — current branch name -- `has_uncommitted` / `has_staged` — whether there are uncommitted changes +- `has_uncommitted` / `has_staged` / `has_untracked` — whether there are uncommitted, staged, or untracked changes If `auth_ok=true`, set `GL_USER` from `auth_user`. @@ -130,8 +142,10 @@ git remote get-url origin | sed -E 's#.*[:/]([^/]+/[^/]+?)(\.git)?$#\1#' Record the result as `UPSTREAM_PROJECT`. Confirm there are actual changes to commit (from the pre-flight output's -`has_uncommitted` or `has_staged` fields, or run `git diff --stat`). If -both are `false`, there are no changes — stop and tell the user. +`has_uncommitted`, `has_staged`, or `has_untracked` fields). If all three +are `false`, there are no changes — stop and tell the user. If +`has_untracked` is `true`, warn the user about untracked files and ask +whether they should be included in the commit. **Pre-flight summary:** Before moving on, you should now know: `UPSTREAM_PROJECT`, which remotes exist, and whether there are changes to commit. You may also know `GL_USER` (if auth is available). @@ -248,11 +262,15 @@ Don't make up details. ### Step 6: Push Use the remote identified during Step 2 (direct push) or Step 3 (fork -workflow) as `PUSH_REMOTE`. Do not hardcode `origin` or `fork` — use the -actual remote name discovered from `git remote -v`. +workflow) as `PUSH_REMOTE`. Set `PUSH_REMOTE` to the actual remote name +discovered from `git remote -v` — typically `origin` for direct push or +`fork` for fork-based workflows: ```bash -../../_shared/scripts/publish.sh push --remote {push-remote} --branch docs/BRANCH_NAME +# Set PUSH_REMOTE based on the push strategy determined in Step 2/3: +# - Direct push: PUSH_REMOTE is the remote pointing to UPSTREAM_PROJECT +# - Fork workflow: PUSH_REMOTE is the remote pointing to FORK_PROJECT +$PUBLISH_SCRIPT push --remote $PUSH_REMOTE --branch docs/BRANCH_NAME ``` **If the script exits with code 3 (push failed):** @@ -273,7 +291,7 @@ exist, build the description (AI-dependent) from the context artifact **Direct push (user has write access):** ```bash -../../_shared/scripts/publish.sh create-mr \ +$PUBLISH_SCRIPT create-mr \ --source docs/BRANCH_NAME \ --target main \ --title "[TICKET_ID]: short description" \ @@ -286,7 +304,7 @@ If no description file exists, use `--description` with inline text instead. **Fork workflow:** ```bash -../../_shared/scripts/publish.sh create-mr \ +$PUBLISH_SCRIPT create-mr \ --project UPSTREAM_PROJECT \ --head FORK_PROJECT \ --source docs/BRANCH_NAME \ diff --git a/e2e/skills/publish.md b/e2e/skills/publish.md index 64c38f31..7ed0dc8c 100644 --- a/e2e/skills/publish.md +++ b/e2e/skills/publish.md @@ -37,6 +37,18 @@ The script provides subcommands: `preflight`, `push`, `check-existing`, ## Process +### Prerequisites: Resolve Script Path + +Before running any subcommands, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Pre-Flight Checks Verify readiness: @@ -63,7 +75,7 @@ Verify readiness: 3. Run the shared pre-flight checks: ```bash - ../../_shared/scripts/publish.sh preflight --platform github + $PUBLISH_SCRIPT preflight --platform github ``` Parse the output to confirm `auth_ok=true` and check for @@ -123,7 +135,7 @@ Confirm with the user before proceeding. ### Step 4: Push Branch ```bash -../../_shared/scripts/publish.sh push --remote origin --branch {branch-name} +$PUBLISH_SCRIPT push --remote origin --branch {branch-name} ``` ### Step 5: Create PR Description @@ -180,10 +192,10 @@ For fork-based workflows, use `{fork-owner}:{branch-name}` as the ```bash # Fork-based: -../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {fork-owner}:{branch-name} +$PUBLISH_SCRIPT check-existing --repo {upstream-owner}/{repo} --head {fork-owner}:{branch-name} # Direct clone: -../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {branch-name} +$PUBLISH_SCRIPT check-existing --repo {upstream-owner}/{repo} --head {branch-name} ``` If exit code is 5, a PR already exists — parse the PR number and URL @@ -195,7 +207,7 @@ stop and report the error. If exit code is 0, create a new PR. `{upstream-owner}/{repo}`): ```bash -../../_shared/scripts/publish.sh create-pr \ +$PUBLISH_SCRIPT create-pr \ --repo {upstream-owner}/{repo} \ --base {pr-target} \ --head {fork-owner}:{branch-name} \ @@ -211,7 +223,7 @@ branch (on the fork). **If the repo is a direct clone** (not a fork): ```bash -../../_shared/scripts/publish.sh create-pr \ +$PUBLISH_SCRIPT create-pr \ --base {pr-target} \ --head {branch-name} \ --title "{issue-key}: {story title}" \ @@ -240,7 +252,7 @@ records the repo that was pushed to. **If the repo is a fork** (set `repo` to the upstream, `origin` to the fork): ```bash -../../_shared/scripts/publish.sh save-metadata \ +$PUBLISH_SCRIPT save-metadata \ --file .artifacts/e2e/{issue-key}/publish-metadata.json \ repo={upstream-owner}/{repo} \ origin={fork-owner}/{repo} \ @@ -254,7 +266,7 @@ records the repo that was pushed to. **If the repo is a direct clone** (`repo` and `origin` are the same): ```bash -../../_shared/scripts/publish.sh save-metadata \ +$PUBLISH_SCRIPT save-metadata \ --file .artifacts/e2e/{issue-key}/publish-metadata.json \ repo={owner}/{repo} \ origin={owner}/{repo} \ diff --git a/implement/skills/publish.md b/implement/skills/publish.md index 89522b84..addc0086 100644 --- a/implement/skills/publish.md +++ b/implement/skills/publish.md @@ -37,6 +37,18 @@ The script provides subcommands: `preflight`, `push`, `check-existing`, ## Process +### Prerequisites: Resolve Script Path + +Before running any subcommands, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Pre-Flight Checks Verify readiness: @@ -63,7 +75,7 @@ Verify readiness: 3. Run the shared pre-flight checks: ```bash - ../../_shared/scripts/publish.sh preflight --platform github + $PUBLISH_SCRIPT preflight --platform github ``` Parse the output to confirm `auth_ok=true` and check for @@ -122,7 +134,7 @@ Confirm with the user before proceeding. ### Step 4: Push Branch ```bash -../../_shared/scripts/publish.sh push --remote origin --branch {branch-name} +$PUBLISH_SCRIPT push --remote origin --branch {branch-name} ``` ### Step 5: Create PR Description @@ -171,7 +183,7 @@ whether this is a fork-based workflow. First, check whether a PR already exists for this branch: ```bash -../../_shared/scripts/publish.sh check-existing --repo {upstream-owner}/{repo} --head {branch-name} +$PUBLISH_SCRIPT check-existing --repo {upstream-owner}/{repo} --head {branch-name} ``` If exit code is 5, a PR already exists — parse the PR number and URL @@ -183,7 +195,7 @@ stop and report the error. If exit code is 0, create a new PR. `{upstream-owner}/{repo}`): ```bash -../../_shared/scripts/publish.sh create-pr \ +$PUBLISH_SCRIPT create-pr \ --repo {upstream-owner}/{repo} \ --base {pr-target} \ --head {fork-owner}:{branch-name} \ @@ -199,7 +211,7 @@ branch (on the fork). **If the repo is a direct clone** (not a fork): ```bash -../../_shared/scripts/publish.sh create-pr \ +$PUBLISH_SCRIPT create-pr \ --base {pr-target} \ --head {branch-name} \ --title "{issue-key}: {story title}" \ @@ -212,7 +224,9 @@ path (e.g., `https://github.com/owner/repo/pull/42` → `42`). If the script exits with code 4 (PR creation failed), fall back to providing the user with a GitHub compare URL: -`https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` + +- Fork-based: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{fork-owner}:{branch-name}?expand=1` +- Direct clone: `https://github.com/{upstream-owner}/{repo}/compare/{pr-target}...{branch-name}?expand=1` ### Step 7: Save Publish Metadata @@ -226,7 +240,7 @@ records the repo that was pushed to. **If the repo is a fork** (set `repo` to the upstream, `origin` to the fork): ```bash -../../_shared/scripts/publish.sh save-metadata \ +$PUBLISH_SCRIPT save-metadata \ --file .artifacts/implement/{issue-key}/publish-metadata.json \ repo={upstream-owner}/{repo} \ origin={fork-owner}/{repo} \ @@ -240,7 +254,7 @@ records the repo that was pushed to. **If the repo is a direct clone** (`repo` and `origin` are the same): ```bash -../../_shared/scripts/publish.sh save-metadata \ +$PUBLISH_SCRIPT save-metadata \ --file .artifacts/implement/{issue-key}/publish-metadata.json \ repo={owner}/{repo} \ origin={owner}/{repo} \ diff --git a/prd/skills/publish.md b/prd/skills/publish.md index 308bc4cb..f896418d 100644 --- a/prd/skills/publish.md +++ b/prd/skills/publish.md @@ -35,17 +35,17 @@ The script provides subcommands: `preflight`, `push`, `check-existing`, ## Process -### Step 0: Resolve Script Path +### Prerequisites: Resolve Script Path Before any `cd` or subshell that changes the working directory, resolve the shared script to an absolute path so it remains valid: ```bash -PUBLISH_SCRIPT="$(cd "$(dirname "../../_shared/scripts/publish.sh")" && pwd)/publish.sh" +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" ``` Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent -commands (Steps 3, 5, 6). +commands. ### Step 1: Read the PRD @@ -134,6 +134,17 @@ for release and feature slug separately. All git operations in this step run against the **docs repo**, not the source repo. Use `git -C "{docs_repo_path}"` for all commands. +Verify the docs repo has no uncommitted, staged, or untracked changes +before modifying it: + +```bash +git -C "{docs_repo_path}" status --porcelain +``` + +If the output is non-empty, the docs repo has local changes. **Stop and +ask the user** how to proceed — they may need to stash or commit those +changes first. Do not copy files into a dirty working tree. + Check if the branch already exists (locally or on the remote) before creating it: ```bash @@ -236,8 +247,20 @@ repo's artifact directory): Determine `{owner}/{repo}` from the `docs_repo_remote` in `.artifacts/config.json` (e.g., `git@github.com:org/planning-docs.git` → `org/planning-docs`), then -create the draft PR. If `{issue-key}` is a Jira key, prefix the title -with it (`{issue-key}: PRD - {title}`); otherwise use `PRD: {title}`. +check for an existing PR before creating one. If `{issue-key}` is a Jira +key, prefix the title with it (`{issue-key}: PRD - {title}`); otherwise +use `PRD: {title}`. + +First, check whether a PR already exists for this branch: + +```bash +"$PUBLISH_SCRIPT" check-existing --repo {owner}/{repo} --head {branch-name} +``` + +If exit code is 5, a PR already exists — skip to Step 6 and report its +URL. Parse the PR number from the returned JSON. If the command fails +(non-zero exit other than 5), stop and report the error. If exit code +is 0, create a new PR: ```bash "$PUBLISH_SCRIPT" create-pr \ From b049d3fe9f905651fc91b6945a1b959277f98fe4 Mon Sep 17 00:00:00 2001 From: Chai Bot <chai-bot@redhat.com> Date: Wed, 9 Sep 2026 18:51:16 +0000 Subject: [PATCH 4/6] fix: address review round 3 findings - NUL-delimited sort in save-metadata to handle newline-containing values safely (publish.sh) - Move PUBLISH_SCRIPT assignment after Step 0 in bugfix/pr.md so git rev-parse resolves from the correct directory - Fork-qualified check-existing head in bugfix/pr.md (FORK_OWNER:bugfix/BRANCH_NAME) - Quote $PUBLISH_SCRIPT in all docs-writer/create-mr.md invocations - Add check-existing before create-mr in docs-writer for GitLab duplicate-prevention, including FORK_PROJECT context - Add has_untracked to dirty-state check and auth_ok=false abort in e2e/publish.md - Show conditional PR title pattern in prd/publish.md ({issue-key}: PRD - {title} vs PRD: {title}) - Fix BRANCH_NAME literal in docs-writer push command to use variable substitution Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> --- _shared/scripts/publish.sh | 4 +++- bugfix/skills/pr.md | 31 +++++++++++++++++-------------- docs-writer/skills/create-mr.md | 30 ++++++++++++++++++++++-------- e2e/skills/publish.md | 9 ++++++--- prd/skills/publish.md | 10 ++++++++++ 5 files changed, 58 insertions(+), 26 deletions(-) diff --git a/_shared/scripts/publish.sh b/_shared/scripts/publish.sh index fcee4ec6..4b6185e0 100755 --- a/_shared/scripts/publish.sh +++ b/_shared/scripts/publish.sh @@ -515,10 +515,12 @@ cmd_save_metadata() { # Build JSON using printf — avoids jq dependency. # Keys are sorted alphabetically for stable output. + # NUL-delimited sort prevents values with embedded newlines from being + # split into separate lines before json_escape can process them. local json="{" local first="true" local -a sorted_pairs - IFS=$'\n' read -r -d '' -a sorted_pairs < <(printf '%s\n' "${pairs[@]}" | sort && printf '\0') || true + IFS= read -r -d '' -a sorted_pairs < <(printf '%s\0' "${pairs[@]}" | sort -z && printf '\0') || true for pair in "${sorted_pairs[@]}"; do local key="${pair%%=*}" diff --git a/bugfix/skills/pr.md b/bugfix/skills/pr.md index 9d95ca0c..c6d30e93 100644 --- a/bugfix/skills/pr.md +++ b/bugfix/skills/pr.md @@ -47,18 +47,6 @@ script. Reference it using a relative path from this file: The script provides subcommands: `preflight`, `push`, `check-existing`, `create-pr`, and `save-metadata`. See the script header for full usage. -### Prerequisites: Resolve Script Path - -Before running any subcommands, resolve the shared script to an -absolute path so it remains valid regardless of working directory: - -```bash -PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" -``` - -Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent -commands. - ## Process ### Placeholders Used in This Skill @@ -92,6 +80,18 @@ commands run from there. If the user provides a path or the repo is obvious from session context (prior commands, artifacts), use that directly. +### Step 0a: Resolve Script Path + +Now that you are inside the project repo, resolve the shared script to an +absolute path so it remains valid regardless of working directory: + +```bash +PUBLISH_SCRIPT="$(git rev-parse --show-toplevel)/_shared/scripts/publish.sh" +``` + +Use `$PUBLISH_SCRIPT` instead of the relative path in all subsequent +commands. + ### Step 1: Pre-flight Checks Run ALL of these before doing anything else. Do not skip any. @@ -471,12 +471,15 @@ access. Please run: `git push -u fork BRANCH_NAME`" ### Step 9: Create the Draft PR -**Check for an existing PR** before attempting creation: +**Check for an existing PR** before attempting creation. Use +`FORK_OWNER:bugfix/BRANCH_NAME` so the check matches only PRs from +this fork (plain `bugfix/BRANCH_NAME` would match any fork's branch +with the same name): ```bash $PUBLISH_SCRIPT check-existing \ --repo UPSTREAM_OWNER/REPO \ - --head bugfix/BRANCH_NAME + --head FORK_OWNER:bugfix/BRANCH_NAME ``` If exit code is 5, a PR already exists — skip to Step 10 and report its diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index 29425f04..65ba3ecd 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -77,7 +77,7 @@ Run ALL of these before doing anything else. Do not skip any. **1a. Run the shared pre-flight checks:** ```bash -$PUBLISH_SCRIPT preflight --platform gitlab +"$PUBLISH_SCRIPT" preflight --platform gitlab ``` Parse the structured output: @@ -270,7 +270,7 @@ discovered from `git remote -v` — typically `origin` for direct push or # Set PUSH_REMOTE based on the push strategy determined in Step 2/3: # - Direct push: PUSH_REMOTE is the remote pointing to UPSTREAM_PROJECT # - Fork workflow: PUSH_REMOTE is the remote pointing to FORK_PROJECT -$PUBLISH_SCRIPT push --remote $PUSH_REMOTE --branch docs/BRANCH_NAME +"$PUBLISH_SCRIPT" push --remote "$PUSH_REMOTE" --branch "docs/$BRANCH_NAME" ``` **If the script exits with code 3 (push failed):** @@ -288,14 +288,28 @@ phase at `.artifacts/${ticket_id}/04-mr-description.md`. If the file does not exist, build the description (AI-dependent) from the context artifact (`01-context.md`) and plan artifact (`02-plan.md`). +**Check for an existing MR** before attempting creation: + +```bash +# Direct push: +"$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "docs/$BRANCH_NAME" --platform gitlab + +# Fork workflow: +"$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head FORK_PROJECT --platform gitlab +``` + +If exit code is 5, an MR already exists — skip to Step 8 and report its +URL. If the command fails (non-zero exit other than 5), stop and report +the error. If exit code is 0, create a new MR: + **Direct push (user has write access):** ```bash -$PUBLISH_SCRIPT create-mr \ - --source docs/BRANCH_NAME \ +"$PUBLISH_SCRIPT" create-mr \ + --source "docs/$BRANCH_NAME" \ --target main \ --title "[TICKET_ID]: short description" \ - --desc-file .artifacts/${ticket_id}/04-mr-description.md \ + --desc-file ".artifacts/${ticket_id}/04-mr-description.md" \ --draft ``` @@ -304,13 +318,13 @@ If no description file exists, use `--description` with inline text instead. **Fork workflow:** ```bash -$PUBLISH_SCRIPT create-mr \ +"$PUBLISH_SCRIPT" create-mr \ --project UPSTREAM_PROJECT \ --head FORK_PROJECT \ - --source docs/BRANCH_NAME \ + --source "docs/$BRANCH_NAME" \ --target main \ --title "[TICKET_ID]: short description" \ - --desc-file .artifacts/${ticket_id}/04-mr-description.md \ + --desc-file ".artifacts/${ticket_id}/04-mr-description.md" \ --draft ``` diff --git a/e2e/skills/publish.md b/e2e/skills/publish.md index 7ed0dc8c..173ce42c 100644 --- a/e2e/skills/publish.md +++ b/e2e/skills/publish.md @@ -78,9 +78,12 @@ Verify readiness: $PUBLISH_SCRIPT preflight --platform github ``` - Parse the output to confirm `auth_ok=true` and check for - `has_uncommitted=true` or `has_staged=true`. If there are uncommitted - changes, ask the user how to proceed. + Parse the output and check: + - `auth_ok` — if `false`, **stop and tell the user** that GitHub CLI + authentication is required to push and create a PR. Suggest running + `gh auth login` and retrying `/publish`. Do not continue without auth. + - `has_uncommitted`, `has_staged`, `has_untracked` — if any are `true`, + ask the user how to proceed (commit, stash, or include untracked files). ### Step 2: Cross-Cutting Review diff --git a/prd/skills/publish.md b/prd/skills/publish.md index f896418d..30765ae8 100644 --- a/prd/skills/publish.md +++ b/prd/skills/publish.md @@ -263,6 +263,7 @@ URL. Parse the PR number from the returned JSON. If the command fails is 0, create a new PR: ```bash +# When {issue-key} is a Jira key (e.g., EDM-1471): "$PUBLISH_SCRIPT" create-pr \ --repo {owner}/{repo} \ --base {base-branch} \ @@ -270,6 +271,15 @@ is 0, create a new PR: --title "{issue-key}: PRD - {title}" \ --body-file .artifacts/prd/{issue-key}/04-pr-description.md \ --draft + +# When no issue key exists: +"$PUBLISH_SCRIPT" create-pr \ + --repo {owner}/{repo} \ + --base {base-branch} \ + --head {branch-name} \ + --title "PRD: {title}" \ + --body-file .artifacts/prd/{issue-key}/04-pr-description.md \ + --draft ``` The script prints the PR URL on stdout. Parse the PR number from the URL path. From 37d073b5289738ad8c1f0315d0f234ee49686bc1 Mon Sep 17 00:00:00 2001 From: Chai Bot <chai-bot@redhat.com> Date: Wed, 9 Sep 2026 19:08:45 +0000 Subject: [PATCH 5/6] fix: address review round 4 findings 1. Use mapfile instead of read for NUL-delimited metadata pairs to read ALL records (read only captured the first one) 2. Quote $PUBLISH_SCRIPT in bugfix/skills/pr.md (5 invocations) 3. Quote $PUBLISH_SCRIPT in e2e/skills/publish.md (8 invocations) 4. Escape metadata keys through json_escape in cmd_save_metadata 5. Fix FORK_PROJECT misuse as --head in docs-writer check-existing; use docs/$BRANCH_NAME (branch name) instead of project path 6. Fold Step 0a into Step 0 in bugfix/skills/pr.md to fix step sequencing Assisted-by: Claude Code <noreply@anthropic.com> --- _shared/scripts/publish.sh | 3 ++- bugfix/skills/pr.md | 12 +++++------- docs-writer/skills/create-mr.md | 2 +- e2e/skills/publish.md | 16 ++++++++-------- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/_shared/scripts/publish.sh b/_shared/scripts/publish.sh index 4b6185e0..c445744c 100755 --- a/_shared/scripts/publish.sh +++ b/_shared/scripts/publish.sh @@ -520,7 +520,7 @@ cmd_save_metadata() { local json="{" local first="true" local -a sorted_pairs - IFS= read -r -d '' -a sorted_pairs < <(printf '%s\0' "${pairs[@]}" | sort -z && printf '\0') || true + mapfile -d '' -t sorted_pairs < <(printf '%s\0' "${pairs[@]}" | sort -z) for pair in "${sorted_pairs[@]}"; do local key="${pair%%=*}" @@ -536,6 +536,7 @@ cmd_save_metadata() { # (e.g., "007" → 7) and to keep the output type-stable. Full JSON # escaping handles newlines, tabs, quotes, backslashes, and control # characters that would otherwise produce invalid JSON. + key=$(json_escape "$key") value=$(json_escape "$value") json+=$(printf '\n "%s": "%s"' "$key" "$value") done diff --git a/bugfix/skills/pr.md b/bugfix/skills/pr.md index c6d30e93..f59f66c3 100644 --- a/bugfix/skills/pr.md +++ b/bugfix/skills/pr.md @@ -80,8 +80,6 @@ commands run from there. If the user provides a path or the repo is obvious from session context (prior commands, artifacts), use that directly. -### Step 0a: Resolve Script Path - Now that you are inside the project repo, resolve the shared script to an absolute path so it remains valid regardless of working directory: @@ -99,7 +97,7 @@ Run ALL of these before doing anything else. Do not skip any. **1a. Run the shared pre-flight checks:** ```bash -$PUBLISH_SCRIPT preflight --platform github +"$PUBLISH_SCRIPT" preflight --platform github ``` Parse the structured output: @@ -455,7 +453,7 @@ to write an accurate commit message. Don't make up details. ### Step 8: Push to Fork ```bash -$PUBLISH_SCRIPT push --remote fork --branch bugfix/BRANCH_NAME +"$PUBLISH_SCRIPT" push --remote fork --branch bugfix/BRANCH_NAME ``` **If the script exits with code 3 (push failed):** @@ -477,7 +475,7 @@ this fork (plain `bugfix/BRANCH_NAME` would match any fork's branch with the same name): ```bash -$PUBLISH_SCRIPT check-existing \ +"$PUBLISH_SCRIPT" check-existing \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME ``` @@ -493,7 +491,7 @@ and report the failure** — do not fall through to PR creation. If the `--body-file` artifact exists: ```bash -$PUBLISH_SCRIPT create-pr \ +"$PUBLISH_SCRIPT" create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ @@ -506,7 +504,7 @@ If the artifact doesn't exist, generate the PR body inline (AI-dependent — see the template in this skill's Notes section) and pass it with `--body`: ```bash -$PUBLISH_SCRIPT create-pr \ +"$PUBLISH_SCRIPT" create-pr \ --repo UPSTREAM_OWNER/REPO \ --head FORK_OWNER:bugfix/BRANCH_NAME \ --base main \ diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index 65ba3ecd..616b728e 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -295,7 +295,7 @@ exist, build the description (AI-dependent) from the context artifact "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "docs/$BRANCH_NAME" --platform gitlab # Fork workflow: -"$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head FORK_PROJECT --platform gitlab +"$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "docs/$BRANCH_NAME" --platform gitlab ``` If exit code is 5, an MR already exists — skip to Step 8 and report its diff --git a/e2e/skills/publish.md b/e2e/skills/publish.md index 173ce42c..22047a7d 100644 --- a/e2e/skills/publish.md +++ b/e2e/skills/publish.md @@ -75,7 +75,7 @@ Verify readiness: 3. Run the shared pre-flight checks: ```bash - $PUBLISH_SCRIPT preflight --platform github + "$PUBLISH_SCRIPT" preflight --platform github ``` Parse the output and check: @@ -138,7 +138,7 @@ Confirm with the user before proceeding. ### Step 4: Push Branch ```bash -$PUBLISH_SCRIPT push --remote origin --branch {branch-name} +"$PUBLISH_SCRIPT" push --remote origin --branch {branch-name} ``` ### Step 5: Create PR Description @@ -195,10 +195,10 @@ For fork-based workflows, use `{fork-owner}:{branch-name}` as the ```bash # Fork-based: -$PUBLISH_SCRIPT check-existing --repo {upstream-owner}/{repo} --head {fork-owner}:{branch-name} +"$PUBLISH_SCRIPT" check-existing --repo {upstream-owner}/{repo} --head {fork-owner}:{branch-name} # Direct clone: -$PUBLISH_SCRIPT check-existing --repo {upstream-owner}/{repo} --head {branch-name} +"$PUBLISH_SCRIPT" check-existing --repo {upstream-owner}/{repo} --head {branch-name} ``` If exit code is 5, a PR already exists — parse the PR number and URL @@ -210,7 +210,7 @@ stop and report the error. If exit code is 0, create a new PR. `{upstream-owner}/{repo}`): ```bash -$PUBLISH_SCRIPT create-pr \ +"$PUBLISH_SCRIPT" create-pr \ --repo {upstream-owner}/{repo} \ --base {pr-target} \ --head {fork-owner}:{branch-name} \ @@ -226,7 +226,7 @@ branch (on the fork). **If the repo is a direct clone** (not a fork): ```bash -$PUBLISH_SCRIPT create-pr \ +"$PUBLISH_SCRIPT" create-pr \ --base {pr-target} \ --head {branch-name} \ --title "{issue-key}: {story title}" \ @@ -255,7 +255,7 @@ records the repo that was pushed to. **If the repo is a fork** (set `repo` to the upstream, `origin` to the fork): ```bash -$PUBLISH_SCRIPT save-metadata \ +"$PUBLISH_SCRIPT" save-metadata \ --file .artifacts/e2e/{issue-key}/publish-metadata.json \ repo={upstream-owner}/{repo} \ origin={fork-owner}/{repo} \ @@ -269,7 +269,7 @@ $PUBLISH_SCRIPT save-metadata \ **If the repo is a direct clone** (`repo` and `origin` are the same): ```bash -$PUBLISH_SCRIPT save-metadata \ +"$PUBLISH_SCRIPT" save-metadata \ --file .artifacts/e2e/{issue-key}/publish-metadata.json \ repo={owner}/{repo} \ origin={owner}/{repo} \ From 6a1ac877c949d84c98bd04fa67336fe2fbe224ea Mon Sep 17 00:00:00 2001 From: Chai Bot <chai-bot@redhat.com> Date: Wed, 9 Sep 2026 19:39:13 +0000 Subject: [PATCH 6/6] fix: harden save-metadata sort and fork-qualify GitLab check-existing Replace the process substitution in save-metadata with a temp-file approach so sort failures are detected and reported. The prior `mapfile < <(... | sort -z)` silently masked sort exit status. Add project:branch format support to the GitLab check-existing path, matching the owner:branch pattern already used for GitHub. The docs-writer fork workflow now passes FORK_PROJECT:branch so the MR lookup filters by source_project_id, preventing false matches from other forks with the same branch name. Assisted-by: Claude Code <noreply@anthropic.com> --- _shared/scripts/publish.sh | 44 ++++++++++++++++++++++++++++++--- docs-writer/skills/create-mr.md | 4 +-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/_shared/scripts/publish.sh b/_shared/scripts/publish.sh index c445744c..647cad28 100755 --- a/_shared/scripts/publish.sh +++ b/_shared/scripts/publish.sh @@ -221,11 +221,13 @@ cmd_push() { # Subcommand: check-existing # --------------------------------------------------------------------------- # Check whether an open PR (GitHub) or MR (GitLab) already exists for -# a branch. Supports owner:branch format for fork-aware matching. +# a branch. Supports owner:branch (GitHub) and project:branch (GitLab) +# formats for fork-aware matching. # # Flags: # --repo <owner/repo> Target repository -# --head <ref> Branch or owner:branch to match +# --head <ref> Branch, owner:branch (GitHub), or +# project:branch (GitLab) to match # --platform github|gitlab Which platform (default: github) # # Exit code 0 if NO existing PR/MR found (safe to create one). @@ -274,6 +276,12 @@ cmd_check_existing() { ;; gitlab) local source_branch="$head" + local source_project="" + if [[ "$head" == *:* ]]; then + # project:branch format — extract source project for cross-fork filtering + source_project="${head%%:*}" + source_branch="${head#*:}" + fi local raw exit_code=0 raw=$(glab mr list --repo "$repo" --source-branch "$source_branch" \ --output json 2>/dev/null) || exit_code=$? @@ -281,7 +289,22 @@ cmd_check_existing() { fail "check-existing: GitLab API query failed (exit $exit_code). Check glab auth status." 1 fi local result - result=$(printf '%s' "$raw" | jq -r '.[0] // empty' 2>/dev/null) + if [[ -n "$source_project" ]]; then + # Resolve the fork's numeric project ID to filter by source_project_id, + # preventing false matches from other forks with the same branch name. + local encoded_project + encoded_project=$(printf '%s' "$source_project" | sed 's|/|%2F|g') + local project_id + project_id=$(glab api "projects/$encoded_project" --jq '.id' 2>/dev/null) || true + if [[ -z "$project_id" ]]; then + fail "check-existing: could not resolve project ID for '$source_project'" 1 + fi + result=$(printf '%s' "$raw" | jq -r --argjson pid "$project_id" \ + '[.[] | select(.source_project_id == $pid)] | .[0] // empty' \ + 2>/dev/null) + else + result=$(printf '%s' "$raw" | jq -r '.[0] // empty' 2>/dev/null) + fi if [[ -n "$result" ]]; then echo "$result" exit 5 @@ -517,10 +540,23 @@ cmd_save_metadata() { # Keys are sorted alphabetically for stable output. # NUL-delimited sort prevents values with embedded newlines from being # split into separate lines before json_escape can process them. + # Uses a temp file so sort failures are detected (process substitution + # masks the sort exit status). local json="{" local first="true" local -a sorted_pairs - mapfile -d '' -t sorted_pairs < <(printf '%s\0' "${pairs[@]}" | sort -z) + local sort_tmp + sort_tmp=$(mktemp) || fail "save-metadata: failed to create temp file for sorting" 1 + if ! printf '%s\0' "${pairs[@]}" > "$sort_tmp"; then + rm -f "$sort_tmp" + fail "save-metadata: failed to write pairs to temp file" 1 + fi + if ! sort -z -o "$sort_tmp" "$sort_tmp"; then + rm -f "$sort_tmp" + fail "save-metadata: sort failed" 1 + fi + mapfile -d '' -t sorted_pairs < "$sort_tmp" + rm -f "$sort_tmp" for pair in "${sorted_pairs[@]}"; do local key="${pair%%=*}" diff --git a/docs-writer/skills/create-mr.md b/docs-writer/skills/create-mr.md index 616b728e..5c4a8cd8 100644 --- a/docs-writer/skills/create-mr.md +++ b/docs-writer/skills/create-mr.md @@ -294,8 +294,8 @@ exist, build the description (AI-dependent) from the context artifact # Direct push: "$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "docs/$BRANCH_NAME" --platform gitlab -# Fork workflow: -"$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "docs/$BRANCH_NAME" --platform gitlab +# Fork workflow (project:branch filters by source project to avoid cross-fork false matches): +"$PUBLISH_SCRIPT" check-existing --repo UPSTREAM_PROJECT --head "FORK_PROJECT:docs/$BRANCH_NAME" --platform gitlab ``` If exit code is 5, an MR already exists — skip to Step 8 and report its