Skip to content

fix(bugfix): comprehensive workflow improvements addressing 9 open issues #90

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.

Sign up for GitHub

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

Merged
jwm4 merged 9 commits into from
Mar 31, 2026
Merged

fix(bugfix): comprehensive workflow improvements addressing 9 open issues #90

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
60a515a
fix(bugfix): comprehensive workflow improvements addressing 9 open is…
jwm4 Mar 28, 2026
1f33145
fix(bugfix): gate assess phase on existing PR with AskUserQuestion
jwm4 Mar 28, 2026
d4e3e21
feat(bugfix): add /summary skill as final workflow phase
jwm4 Mar 28, 2026
0e5468e
fix(bugfix): improve PR skill description for better discoverability
jwm4 Mar 28, 2026
9d9a0fc
fix(bugfix): address CodeRabbit review findings
jwm4 Mar 28, 2026
99db059
fix(bugfix): add expired token recovery to PR skill Step 0
jwm4 Mar 28, 2026
2e24cd5
fix(bugfix): clarify test suite commands are per-language
jwm4 Mar 28, 2026
51342d4
fix(bugfix): address CodeRabbit review — stale refs, rebase order, ga…
jwm4 Mar 29, 2026
44f926e
fix(bugfix): use UPSTREAM_REMOTE in unauthenticated fallback
jwm4 Mar 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 60 additions & 5 deletions workflows/bugfix/.claude/skills/assess/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,49 @@ gh repo clone OWNER/REPO /workspace/repos/REPO

This is read-only exploration — understand the code, don't change it.

### Step 3: Summarize Your Understanding
### Step 3: Check for Existing Work

Before investing effort, check whether this bug is already being addressed:

- **Check for linked PRs on the issue:**

```bash
gh issue view NUMBER --repo OWNER/REPO --json body,comments --jq '[.body, .comments[].body] | join("\n")' | grep -i "pull\|PR\|#"
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Refine the grep pattern to reduce false positives.

The pattern "pull\|PR\|#" is very broad and will match:

  • Any markdown heading (#)
  • Partial word matches ("PR" in "PRIORITIZE")
  • Generic uses of "pull" unrelated to PRs
🔍 Suggested pattern refinement 
-gh issue view NUMBER --repo OWNER/REPO --json body,comments --jq '[.body, .comments[].body] | join("\n")' | grep -i "pull\|PR\|#"
+gh issue view NUMBER --repo OWNER/REPO --json body,comments --jq '[.body, .comments[].body] | join("\n")' | grep -iE '#[0-9]+|PR #[0-9]+|pull request #[0-9]+'

This matches actual PR/issue references like #123, PR #123, or `pull request `#123 rather than arbitrary occurrences.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
gh issue view NUMBER --repo OWNER/REPO --json body,comments --jq '[.body, .comments[].body] | join("\n")' | grep -i "pull\|PR\|#"
gh issue view NUMBER --repo OWNER/REPO --json body,comments --jq '[.body, .comments[].body] | join("\n")' | grep -iE '#[0-9]+|PR #[0-9]+|pull request #[0-9]+'
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@workflows/bugfix/.claude/skills/assess/SKILL.md` at line 78, The existing
grep pattern "pull\|PR\|#" is too broad; update the grep invocation in the
pipeline command to use a stricter regex (and -E for extended regex) that only
matches explicit PR/issue references, e.g. replace the pattern with something
like
'(^|[^A-Za-z0-9])(#\d+|PR[[:space:]]*#\d+|pull[[:space:]]+request[[:space:]]*#\d+)([^A-Za-z0-9]|$)'
so the command that contains the current "pull\|PR\|#" pattern only returns real
references like "#123", "PR `#123`" or "pull request `#123`".

```

- **Scan recent PR titles for overlap:**

```bash
gh pr list --repo OWNER/REPO --state open --limit 30 --json number,title,headRefName --jq '.[] | "\(.number)\t\(.title)"'
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider documenting the rationale for --limit 30.

The hardcoded limit of 30 for both PR and issue listings is reasonable for most repositories, but relevant work could be missed in high-activity projects. Consider adding a comment explaining this trade-off, or note that users can manually check if they suspect additional relevant work exists.

Also applies to: 93-93

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@workflows/bugfix/.claude/skills/assess/SKILL.md` at line 84, The hardcoded
"--limit 30" in the gh pr list and corresponding issue listing command should be
justified or made configurable; update the SKILL.md around the gh pr list
--limit 30 invocation (and the duplicate at the other occurrence) to either add
a short comment explaining the trade-off (fast listing vs missing PRs in
high-activity repos) or document how to change the limit (e.g., run the command
with a different --limit or remove it), and ensure both occurrences mention the
rationale and how to adjust the limit so readers know why 30 was chosen and how
to increase it when needed.

```

Skim the titles for anything related to the bug's component, error, or
symptoms. If a PR looks relevant, read its description before proceeding.

- **Check for duplicate or related issues:**

```bash
gh issue list --repo OWNER/REPO --state open --limit 30 --json number,title --jq '.[] | "\(.number)\t\(.title)"'
```

If you find an open PR that appears to directly address this bug, **stop here
and use `AskUserQuestion`** before continuing the assessment. Present the
options:

- "PR #N appears to address this bug — review it instead of starting fresh"
- "PR #N is related but doesn't fully cover it — continue with assessment"
- "Not sure if PR #N is relevant — continue with assessment"

This gate applies in both normal and speedrun mode. Do not continue to Step 4
until the user responds. The `AskUserQuestion` tool triggers platform
notifications so the user knows you need their input.
Comment on lines +96 to +106
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check for plain-text wait instructions instead of AskUserQuestion in skill files.

# Search for wait/stop patterns that should be replaced with AskUserQuestion
rg -n -C3 --type=md 'stop.*wait|wait.*confirm|wait.*user.*respond|pause.*user' workflows/bugfix/.claude/skills/ | grep -v AskUserQuestion

Repository: ambient-code/workflows

Length of output: 1932

The assess skill correctly uses AskUserQuestion; update pr/SKILL.md to align with the same gate requirement.

The assess phase at lines 96-106 properly uses the AskUserQuestion tool for its hard gate, meeting the controller requirement. However, pr/SKILL.md lines 305-306 still use plain text "Stop again and wait for the user to confirm" instead of the AskUserQuestion tool. Both phase gates must use AskUserQuestion to trigger platform notifications consistently.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@workflows/bugfix/.claude/skills/assess/SKILL.md` around lines 96 - 106, The
pr phase uses plain text "Stop again and wait for the user to confirm" instead
of invoking the AskUserQuestion tool; update the pr phase SKILL.md to call
AskUserQuestion with the same three options used in the assess skill ("PR `#N`
appears to address this bug — review it instead of starting fresh", "PR `#N` is
related but doesn't fully cover it — continue with assessment", "Not sure if PR
`#N` is relevant — continue with assessment") and enforce the hard gate so the
flow does not proceed until AskUserQuestion returns the user's choice.


If duplicate or related issues are found (but no PR), note them in the
assessment and continue — these inform the assessment but don't block it.

If no existing work is found, note that and continue.

### Step 4: Summarize Your Understanding

Present a clear, concise summary to the user covering:

Expand All @@ -81,7 +123,7 @@ Present a clear, concise summary to the user covering:
- **Severity/impact:** Your assessment of how serious this is, based on the
information available

### Step 4: Identify What You Know vs. What's Missing
### Step 5: Identify What You Know vs. What's Missing

Be explicit about gaps:

Expand All @@ -93,7 +135,7 @@ Be explicit about gaps:
- **Assumptions:** Any assumptions you're making — call them out so the user
can confirm or correct them

### Step 5: Propose a Reproduction Plan
### Step 6: Propose a Reproduction Plan

Based on your understanding, outline how you would approach reproduction:

Expand All @@ -105,7 +147,7 @@ Based on your understanding, outline how you would approach reproduction:
If the bug seems straightforward, the plan can be brief. If it's complex or
ambiguous, be thorough.

### Step 6: Present to the User
### Step 7: Present to the User

Deliver your assessment in this structure:

Expand All @@ -115,6 +157,9 @@ Deliver your assessment in this structure:
**Issue:** [title or one-line summary]
**Source:** [issue URL, conversation, etc.]

### Existing Work
[Any related PRs, duplicate issues, or prior attempts — or "None found"]

### Understanding
[Your 2-3 sentence summary of the bug]

Expand All @@ -139,7 +184,17 @@ Deliver your assessment in this structure:
Be direct. If the bug report is clear and complete, say so. If it's vague or
missing critical details, say that too.

### Step 7: Write the Assessment Artifact
**If the bug doesn't actually apply**, say so clearly and present options:

- "This issue doesn't affect your project — here's why. Want to close it?"
- "The reported issue doesn't apply directly, but here's a related improvement
we could make (with trade-offs): ..."
- "This appears to be a duplicate of #N — should we close this one?"

Do not proceed to fix something you've concluded isn't broken. Present your
finding and let the user decide.

### Step 8: Write the Assessment Artifact

Save your assessment to `artifacts/bugfix/reports/assessment.md` so that
subsequent phases (and speedrun resumption) can detect that this phase is
Expand Down
33 changes: 28 additions & 5 deletions workflows/bugfix/.claude/skills/controller/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ executing phases and handling transitions between them.
8. **PR** (`/pr`) — `.claude/skills/pr/SKILL.md`
Push the branch to a fork and create a draft pull request.

9. **Summary** (`/summary`) — `.claude/skills/summary/SKILL.md`
Scan all artifacts and present a synthesized summary of findings, decisions,
and status. Can also be invoked at any point mid-workflow.

Phases can be skipped or reordered at the user's discretion.

## How to Execute a Phase
Expand All @@ -43,12 +47,20 @@ Phases can be skipped or reordered at the user's discretion.
"Starting the /fix phase (dispatched by `.claude/skills/controller/SKILL.md`)."
This is very important so the user knows the workflow is working, learns
about the available phases, and so skills can find their way back here.
2. **Read** the skill file from the list above
2. **Read** the skill file from the list above. You MUST call the Read tool on
the skill's `SKILL.md` file before executing. If you find yourself executing
a phase without having read its skill file, you are doing it wrong — stop
and read it now.
3. **Execute** the skill's steps directly — the user should see your progress
4. When the skill is done, it will report its findings and re-read this
controller. Then use "Recommending Next Steps" below to offer options.
5. Present the skill's results and your recommendations to the user
6. **Stop and wait** for the user to tell you what to do next
5. Present the skill's results and your recommendations to the user.
6. **Use `AskUserQuestion` to get the user's decision.** Present the
recommended next step and alternatives as options. Do NOT continue until the
user responds. This is a hard gate — the `AskUserQuestion` tool triggers
platform notifications and status indicators so the user knows you need
their input. Plain-text questions do not create these signals and the user
may not see them.

## Recommending Next Steps

Expand All @@ -59,7 +71,7 @@ happened.
### Typical Flow

```text
assess → reproduce → diagnose → fix → test → review → document → pr
assess → reproduce → diagnose → fix → test → review → document → pr → summary
```

### What to Recommend
Expand Down Expand Up @@ -110,6 +122,17 @@ directly — don't force them through earlier phases.

## Rules

- **Never auto-advance.** Always wait for the user between phases.
- **Never auto-advance.** Always use `AskUserQuestion` and wait for the user's
response between phases. This is the single most important rule in this
controller. If you proceed to another phase without the user's explicit
go-ahead, the workflow is broken.
- **Always read skill files.** Every phase execution MUST begin with a Read
tool call on the skill's `SKILL.md` file. Do not execute a phase from memory
or general knowledge — the skill files contain specific, tested instructions
that differ from what you might do ad-hoc.
- **Urgency does not bypass process.** Security advisories, critical bugs, and
production incidents may create pressure to act fast. The phase-gated
workflow exists precisely to prevent hasty action. Follow every phase gate
regardless of perceived urgency.
- **Recommendations come from this file, not from skills.** Skills report
findings; this controller decides what to recommend next.
29 changes: 25 additions & 4 deletions workflows/bugfix/.claude/skills/fix/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,29 +59,50 @@ Before finalizing the implementation, ensure thoroughness:
- **Check for complete enumeration**: If implementing switch/case logic or conditional checks, verify you've handled all possible values. Search the codebase for where these values are defined or used.
- **Example**: If implementing polling that stops on "terminal" session phases, search the codebase for all usages of session phases to build a complete list (Stopped, Completed, Failed, Error) rather than assuming you know them all.

### Step 4: Address Related Code
### Step 4: Review Error Handling UX

If your fix involves error handling, validation, or user-facing messages,
review the error paths for clarity:

- **Match error context to error type.** A CLI argument error should use the
CLI framework's error type (e.g., `click.BadParameter`), while a
configuration file error should use a general exception that says which file
and line caused the problem. Don't report config file errors as CLI parameter
errors, or vice versa.
- **Test every error path manually.** Trigger each error condition and read the
message from the user's perspective. Is it clear what went wrong? Does it
point to the right place to fix it?
- **Consider different error contexts:**
- CLI errors → should reference the flag or argument
- Config file errors → should reference the file path and setting
- Runtime errors → should include enough context to reproduce
- API errors → should include the endpoint and status code
- **Ensure error messages don't leak internals.** Stack traces, internal paths,
and raw exception types are useful for developers but confusing for users.

### Step 5: Address Related Code

- Fix similar patterns identified in root cause analysis
- Update affected function signatures if necessary
- Ensure consistency across the codebase
- Consider adding defensive programming where appropriate

### Step 5: Update Documentation
### Step 6: Update Documentation

- Update inline code documentation
- Modify API documentation if interfaces changed
- Update configuration documentation if settings changed
- Note any breaking changes clearly

### Step 6: Pre-commit Quality Checks
### Step 7: Pre-commit Quality Checks

- Run code formatters (e.g., `gofmt`, `black`, `prettier`)
- Run linters and fix all warnings (e.g., `golangci-lint`, `flake8`, `eslint`)
- Ensure code compiles/builds without errors
- Check for any new security vulnerabilities introduced
- Verify no secrets or sensitive data added

### Step 7: Document Implementation
### Step 8: Document Implementation

Create `artifacts/bugfix/fixes/implementation-notes.md` containing:

Expand Down
Loading
Loading