-
Notifications
You must be signed in to change notification settings - Fork 34
feat(dev-team): sync PR skill with latest bugfix workflow improvements #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,20 @@ you back here with the proper workflow context. | |
|
|
||
| --- | ||
|
|
||
| You are preparing to submit a bug fix as a pull request. This skill provides a | ||
| ## Workflow Context | ||
|
|
||
| This skill is workflow-agnostic. The following values are set by the calling | ||
| workflow and used throughout this document: | ||
|
|
||
| | Variable | Value | | ||
| | --- | --- | | ||
| | `WORKFLOW_NAME` | `bugfix` | | ||
| | `ARTIFACT_DIR` | `artifacts/bugfix/` | | ||
| | `COORDINATOR_SKILL` | `.claude/skills/controller/SKILL.md` | | ||
|
|
||
| --- | ||
|
|
||
| You are preparing to submit changes as a pull request. This skill provides a | ||
| systematic, failure-resistant process for getting code from the working directory | ||
| into a PR. It handles the common obstacles: authentication, fork workflows, | ||
| remote configuration, and cross-repo PR creation. | ||
|
|
@@ -27,7 +40,7 @@ fallback ladder at the bottom of this file. | |
|
|
||
| ## Your Role | ||
|
|
||
| Get the bug fix changes submitted as a draft pull request. Handle the full | ||
| Get the changes submitted as a draft pull request. Handle the full | ||
| git workflow: branch, commit, push, and PR creation. When steps fail, follow | ||
| the documented recovery paths instead of guessing. | ||
|
|
||
|
|
@@ -59,7 +72,7 @@ These are determined during pre-flight checks. Record each value as you go. | |
| | `FORK_OWNER` | Step 3: owner portion of fork's `nameWithOwner`, or `GH_USER` if newly created | `jsmith` | | ||
| | `FORK_REMOTE` | Step 4: the git remote name pointing to the fork | `fork` / `origin` | | ||
| | `REPO` | The repository name (without owner) | `myproject` | | ||
| | `BRANCH_NAME` | Step 5: the branch you create | `bugfix/issue-42-null-check` | | ||
| | `BRANCH_NAME` | Step 5: the branch you create | `feature/issue-42-auth-middleware` | | ||
|
|
||
| ### Step 0: Determine Auth Context | ||
|
|
||
|
|
@@ -129,7 +142,7 @@ checks above to set `AUTH_TYPE` and `GH_USER`. | |
|
|
||
| ### Step 1: Locate the Project Repository | ||
|
|
||
| The bugfix workflow runs from the workflow directory, but the code changes live | ||
| The workflow runs from the workflow directory, but the code changes live | ||
| in the project repository. Before doing any git work: | ||
|
|
||
| ```bash | ||
|
|
@@ -163,7 +176,7 @@ git config user.email "[email protected]" | |
| ``` | ||
|
|
||
| - If missing and `gh` is NOT authenticated: set reasonable defaults so commits | ||
| work. Use `"bugfix-workflow"` / `"bugfix@workflow.local"` as placeholders. | ||
| work. Use `"workflow-agent"` / `"workflow@agent.local"` as placeholders. | ||
|
|
||
| **2b. Inventory existing remotes:** | ||
|
|
||
|
|
@@ -461,13 +474,17 @@ git rebase FORK_REMOTE/DEFAULT_BRANCH | |
| git checkout -b BRANCH_NAME | ||
| ``` | ||
|
|
||
| Branch naming conventions: | ||
| Branch naming conventions — choose the prefix that matches the work type: | ||
|
|
||
| - `bugfix/issue-NUMBER-SHORT_DESCRIPTION` — bug fixes | ||
| - `feature/issue-NUMBER-SHORT_DESCRIPTION` — new features | ||
| - `refactor/SHORT_DESCRIPTION` — refactoring | ||
| - `docs/SHORT_DESCRIPTION` — documentation changes | ||
|
|
||
| - `bugfix/issue-NUMBER-SHORT_DESCRIPTION` if there's an issue number | ||
| - `bugfix/SHORT_DESCRIPTION` if there's no issue number | ||
| - Use kebab-case, keep it under 50 characters | ||
| Use kebab-case, keep it under 50 characters. Include the issue number when one | ||
| exists. | ||
|
|
||
| If a branch already exists with the changes (from a prior `/fix` phase), use | ||
| If a branch already exists with the changes (from a prior phase), use | ||
| it instead of creating a new one. | ||
|
|
||
| ### Step 6: Stage and Commit | ||
|
|
@@ -476,22 +493,24 @@ Stage changes selectively (`git add path/to/files`, not `git add .`), review | |
| with `git status`, then commit using conventional commit format: | ||
|
|
||
| ```bash | ||
| git commit -m "fix(SCOPE): SHORT_DESCRIPTION | ||
| git commit -m "TYPE(SCOPE): SHORT_DESCRIPTION | ||
|
|
||
| DETAILED_DESCRIPTION | ||
|
|
||
| Fixes #ISSUE_NUMBER" | ||
| ``` | ||
|
|
||
| Use prior artifacts (root cause analysis, implementation notes) to write an | ||
| Where `TYPE` matches the work: `fix`, `feat`, `refactor`, `docs`, `test`, etc. | ||
|
|
||
| Use prior artifacts (analysis, implementation notes) to write an | ||
| accurate commit message. Don't make up details. | ||
|
|
||
| **Include the PR description in the commit body.** When a PR has a single | ||
| commit, GitHub auto-fills the PR description from the commit message. This | ||
| ensures the PR form is pre-populated even when `gh pr create` fails (a | ||
| common case for bot environments). If `artifacts/bugfix/docs/pr-description.md` | ||
| common case for bot environments). If `ARTIFACT_DIR/docs/pr-description.md` | ||
| exists, append its content after the `Fixes #N` line. If it doesn't exist, | ||
| compose a brief PR body from session context (problem, root cause, fix, testing) | ||
| compose a brief PR body from session context (problem, approach, testing) | ||
| and include that instead. | ||
|
Comment on lines
+490
to
494
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check all artifact references in bugfix workflow
echo "=== Document skill output paths ==="
rg -n "pr-description\.md" workflows/bugfix/.claude/skills/document/SKILL.md
echo ""
echo "=== Speedrun skill artifact checks ==="
rg -n "pr-description\.md" workflows/bugfix/.claude/skills/speedrun/SKILL.md
echo ""
echo "=== Any ARTIFACT_DIR definitions ==="
rg -n "ARTIFACT_DIR" workflows/bugfix/.claude/Repository: ambient-code/workflows Length of output: 410 🏁 Script executed: #!/bin/bash
# Examine the PR skill to understand artifact directory resolution
echo "=== Lines 490-495 (context from review) ==="
sed -n '485,500p' workflows/bugfix/.claude/skills/pr/SKILL.md
echo ""
echo "=== Lines 545-555 (context) ==="
sed -n '545,555p' workflows/bugfix/.claude/skills/pr/SKILL.md
echo ""
echo "=== Lines 605-615 (context) ==="
sed -n '605,615p' workflows/bugfix/.claude/skills/pr/SKILL.md
echo ""
echo "=== Search for how artifact directory is determined ==="
rg -n "artifact" workflows/bugfix/.claude/skills/pr/SKILL.md | head -20
echo ""
echo "=== Check for environment variables or path construction ==="
rg -n "ARTIFACT|artifact.*dir|artifact.*path" workflows/bugfix/.claude/skills/pr/SKILL.md -A 2Repository: ambient-code/workflows Length of output: 4218 🏁 Script executed: #!/bin/bash
# Check if there's implementation code or config showing artifact directory resolution
echo "=== Search for workflow name or artifact path variables ==="
rg -n "workflow|artifact.*directory|/artifacts/" workflows/bugfix/.claude/skills/pr/SKILL.md | head -30
echo ""
echo "=== Check for context or metadata that might define artifact dir ==="
rg -n "context|metadata|input|parameter" workflows/bugfix/.claude/skills/pr/SKILL.md | head -20
echo ""
echo "=== Check beginning of skill for inputs/outputs ==="
head -50 workflows/bugfix/.claude/skills/pr/SKILL.mdRepository: ambient-code/workflows Length of output: 5688 🏁 Script executed: #!/bin/bash
# Check if PR skill is shared across workflows or specific to bugfix
echo "=== PR skill locations ==="
find workflows -name "pr" -type d | grep -i skill
echo ""
echo "=== Check if dev-team has a PR skill ==="
ls -la workflows/dev-team/.claude/skills/ 2>/dev/null | grep -i pr
echo ""
echo "=== Examine full path context around line 129 in PR skill ==="
sed -n '120,145p' workflows/bugfix/.claude/skills/pr/SKILL.md
echo ""
echo "=== Check if there's any reference to 'bugfix' or workflow name ==="
rg -n "bugfix" workflows/bugfix/.claude/skills/pr/SKILL.mdRepository: ambient-code/workflows Length of output: 1369 🏁 Script executed: #!/bin/bash
# Check if the PR skills are identical or workflow-specific
echo "=== Diff PR skills to see if they're identical ==="
diff -u workflows/bugfix/.claude/skills/pr/SKILL.md workflows/dev-team/.claude/skills/pr/SKILL.md | head -50
echo ""
echo "=== Check how artifact directory is actually resolved (search for implementation) ==="
rg -n "docs/pr-description" workflows/bugfix/.claude/skills/pr/SKILL.md -B 5 -A 5
echo ""
echo "=== Check speedrun's reference to artifact ==="
cat workflows/bugfix/.claude/skills/speedrun/SKILL.md | grep -A 10 -B 10 "pr-description.md"Repository: ambient-code/workflows Length of output: 4625 🏁 Script executed: #!/bin/bash
# Verify the actual execution context of the PR skill
echo "=== Check if PR skill execution context is documented ==="
grep -n "cd " workflows/bugfix/.claude/skills/pr/SKILL.md | head -10
echo ""
echo "=== Full context around Step 1 (line 129) where project repo is determined ==="
sed -n '124,160p' workflows/bugfix/.claude/skills/pr/SKILL.md
echo ""
echo "=== Verify: does document skill explicitly state its output location ==="
sed -n '85,115p' workflows/bugfix/.claude/skills/document/SKILL.mdRepository: ambient-code/workflows Length of output: 2303 Critical: This change breaks the bugfix workflow's document → PR skill handoff due to artifact path mismatch. The bugfix workflow's document skill explicitly writes PR descriptions to The phrase "workflow's artifact directory" is used throughout the PR skill (lines 490, 548, 609, 630, 638, 680–681) but is never defined or resolved. This will cause the PR skill to fail to locate the artifact when invoked after the document phase. Before merging, resolve the artifact path:
🤖 Prompt for AI Agents |
||
|
|
||
| ### Step 7: Push to Fork | ||
|
|
@@ -532,8 +551,8 @@ gh pr create \ | |
| --repo UPSTREAM_OWNER/REPO \ | ||
| --head FORK_OWNER:BRANCH_NAME \ | ||
| --base DEFAULT_BRANCH \ | ||
| --title "fix(SCOPE): SHORT_DESCRIPTION" \ | ||
| --body-file artifacts/bugfix/docs/pr-description.md | ||
| --title "TYPE(SCOPE): SHORT_DESCRIPTION" \ | ||
| --body-file ARTIFACT_DIR/docs/pr-description.md | ||
| ``` | ||
|
|
||
| `--head` must be `FORK_OWNER:BRANCH_NAME` format (with the owner prefix) for | ||
|
|
@@ -545,7 +564,7 @@ composed from session artifacts. | |
| This is a common and 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: | ||
|
|
||
| 1. **Write the PR description** to `artifacts/bugfix/docs/pr-description.md` | ||
| 1. **Write the PR description** to `ARTIFACT_DIR/docs/pr-description.md` | ||
| (if not already written). | ||
|
|
||
| 2. **Give the user a pre-filled GitHub compare URL** with `title` and `body` | ||
|
|
@@ -607,7 +626,7 @@ Diagnose it using the Error Recovery table and retry. | |
| If `gh pr create` fails but the branch is pushed to the fork (this is a | ||
| **common and expected** outcome when running as a GitHub App bot): | ||
|
|
||
| 1. **Write the PR body** to `artifacts/bugfix/docs/pr-description.md` | ||
| 1. **Write the PR body** to `ARTIFACT_DIR/docs/pr-description.md` | ||
| 2. **Provide the compare URL with `title` and `body` query params** so the | ||
| PR form opens fully populated (see Step 8 failure path for format) | ||
| 3. **Provide clone-and-checkout commands** for local testing | ||
|
|
@@ -627,16 +646,16 @@ If no fork exists and automated forking fails: | |
| Only if ALL of the above fail — for example, the user has no GitHub account, | ||
| or network access is completely blocked: | ||
|
|
||
| 1. Generate a patch: `git diff > bugfix.patch` | ||
| 2. Write it to `artifacts/bugfix/bugfix.patch` | ||
| 3. Explain to the user how to apply it: `git apply bugfix.patch` | ||
| 1. Generate a patch: `git diff > changes.patch` | ||
| 2. Write it to `ARTIFACT_DIR/changes.patch` | ||
| 3. Explain to the user how to apply it: `git apply changes.patch` | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| 4. **Acknowledge this is a degraded experience** and explain what prevented | ||
| the normal flow | ||
|
|
||
| ## Output | ||
|
|
||
| - The PR URL (printed to the user) | ||
| - Optionally updates `artifacts/bugfix/docs/pr-description.md` if it didn't | ||
| - Optionally updates `ARTIFACT_DIR/docs/pr-description.md` if it didn't | ||
| already exist | ||
|
|
||
| ## Usage Examples | ||
|
|
@@ -676,13 +695,13 @@ or network access is completely blocked: | |
|
|
||
| ## Notes | ||
|
|
||
| - This skill assumes the bug fix work (code changes, tests) is already done. | ||
| Run `/fix` and `/test` first. | ||
| - If `/document` was run, the PR description artifact should already exist at | ||
| `artifacts/bugfix/docs/pr-description.md`. This skill will use it. | ||
| - If `/document` was NOT run, this skill creates a minimal PR body from | ||
| session context (conversation history, prior artifacts). | ||
| - The fork workflow is the standard for open source contributions. Even if the | ||
| - This skill assumes the implementation work (code changes, tests) is already | ||
| done before invoking `/pr`. | ||
| - If a PR description artifact exists at `ARTIFACT_DIR/docs/pr-description.md`, | ||
| this skill will use it. | ||
| - If no PR description artifact exists, this skill creates a minimal PR body | ||
| from session context (conversation history, prior artifacts). | ||
| - The fork workflow is the standard for open-source contributions. Even if the | ||
| user has write access to upstream, using a fork keeps the upstream clean. | ||
|
|
||
| ## When This Phase Is Done | ||
|
|
@@ -693,4 +712,5 @@ Report your results: | |
| - What was included | ||
| - Any follow-up actions needed (mark ready for review, add reviewers, etc.) | ||
|
|
||
| Then announce which file you are returning to (e.g., "Returning to `.claude/skills/controller/SKILL.md`." or "Returning to `.claude/skills/speedrun/SKILL.md` for next phase.") and **re-read that file** for next-step guidance. | ||
| Then announce that you are returning to `COORDINATOR_SKILL` and **re-read | ||
| that file** for next-step guidance. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 321
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 2266
🏁 Script executed:
cat -n workflows/bugfix/.claude/skills/controller/SKILL.md | head -250Repository: ambient-code/workflows
Length of output: 6826
🏁 Script executed:
rg -n "WORKFLOW_NAME\s*=" workflows/bugfix/.claude/Repository: ambient-code/workflows
Length of output: 48
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 121
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 4342
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 652
🏁 Script executed:
rg -i "workflow_name|artifact_dir|WORKFLOW_NAME|ARTIFACT_DIR" workflows/bugfix/ -t md -t yaml -A 2Repository: ambient-code/workflows
Length of output: 48
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 12614
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 3258
🏁 Script executed:
Repository: ambient-code/workflows
Length of output: 48
🏁 Script executed:
rg -B 5 -A 5 "set by the calling workflow" workflows/bugfix/ -t mdRepository: ambient-code/workflows
Length of output: 48
Establish explicit context values in controller before dispatching to PR skill.
The PR skill (lines 16–26) documents that
WORKFLOW_NAME,ARTIFACT_DIR, andCOORDINATOR_SKILLare "set by the calling workflow," but the controller skill does not explicitly initialize or confirm these values before directing users to the PR phase. This creates ambiguity about whether these are template variables to be substituted externally, environment-injected values, or hardcoded literals. The controller should explicitly establish these context values early (e.g., "For this workflow:WORKFLOW_NAME=bugfix,ARTIFACT_DIR=artifacts/bugfix/,COORDINATOR_SKILL=.claude/skills/controller/SKILL.md") to ensure downstream skills receive properly initialized context.🤖 Prompt for AI Agents