Pr Review Fix

PR Review & Fix

Systematically analyze open pull requests for CI failures, code review feedback, and code quality issues — then fix them efficiently.

When to use this skill

Use this skill when you need to:

• Check the status of all open PRs (CI, reviews, conflicts)
• Triage and fix CI build/test failures on PR branches
• Address code review feedback (reviewer comments, requested changes)
• Run a scheduled health check across all open PRs
• Fix multiple PRs in a single session without losing context

Do NOT use for:

• Creating new PRs or new features
• Merging PRs (that's a manual decision)
• General code refactoring unrelated to PR feedback
• Reviewing code as a reviewer (this skill is for responding to reviews)

Workflow

Phase 1 — Discovery

1. Read references/discovery.md for the full discovery procedure. 2. Fetch the list of open PRs from GitHub:

   gh pr list --state open --json number,title,headRefName,statusCheckRollup,reviewDecision,mergeable --limit 30

3. For each PR, classify its health status:

• 🔴 CI Failed — at least one required check failed
• 🟡 Changes Requested — reviewer left requested changes
• 🟢 Healthy — CI passing + approved or no review yet
• ⚪ Conflict — merge conflicts detected

Phase 2 — Triage

1. Read references/triage.md for prioritization rules. 2. Prioritize by severity: CI failures > review changes > conflicts. 3. For each failing PR, identify root cause category:

• Build error — TypeScript/webpack compilation failure
• Test failure — vitest/jest test assertion or timeout
• Lint/type error — ESLint, type-check, or format issues
• Review feedback — code style, logic, security, or design concerns

4. Present a summary table to the user before proceeding to fixes.

Phase 3 — Fix

1. Read references/fix-workflow.md for the fix procedure. 2. For each PR to fix (in priority order): a. Stash current work: git stash b. Check out the PR branch: git checkout -B <branch> github/<branch> c. Reproduce the issue locally (build, test, or lint) d. Apply the fix e. Verify locally: build → test → lint f. Commit with conventional-changelog format: fix(<scope>): 🔧 <description> g. Push: git push github <branch> h. Return to original branch: git checkout <original> && git stash pop 3. After all fixes, present a completion summary.

Phase 4 — Verify

1. After pushing fixes, wait 1-2 minutes for CI to trigger. 2. Check CI status for each fixed PR:

   gh pr checks <number>

3. If CI still fails, loop back to Phase 3 for that PR.

Routing

Git safety rules

• Never force-push to a PR branch unless explicitly asked.
• Never amend commits that are already pushed.
• Always stash before switching branches.
• Always verify build + test locally before pushing.
• One commit per fix session — keep the diff reviewable.

Commit conventions

Follow the project's conventional-changelog format:

fix(<scope>): 🔧 <english description>

Where <scope> is the affected module (e.g., cloudrun, security, code-quality, test).

Minimum self-check

• Did you fetch the latest remote state before analyzing?
• Did you reproduce the failure locally before attempting a fix?
• Did you verify build + test pass after applying the fix?
• Did you switch back to the original branch after each fix?
• Did you present a clear summary of what was fixed and what remains?

CI Pipeline Reference

Project CI overview

This project uses GitHub Actions with the following workflows:

nightly-build.yaml — Publish MCP Package to pkg.pr.new

Triggers:

• Push to main
• Pull requests targeting main
• Manual dispatch

Job: `build-and-publish`

Environment variables (test step):

• TENCENTCLOUD_SECRETID — from secrets
• TENCENTCLOUD_SECRETKEY — from secrets
• CLOUDBASE_ENV_ID — from secrets

Other workflows

Local reproduction

To match CI locally:

cd mcp
npm ci                    # not npm install — match lockfile exactly
npm run build             # webpack --config webpack/index.cjs --mode=production
npm run test              # vitest

Build system

• Bundler: Webpack (config at mcp/webpack/index.cjs)
• Mode: Production
• Source: TypeScript in mcp/src/
• Output: Bundled JS

Test system

• Runner: Vitest
• Config: mcp/vitest.config.*
• Tests: tests/ directory
• Environment-dependent tests: Use test.skipIf(!process.env.CLOUDBASE_ENV_ID) pattern

Common CI vs local differences

PR Discovery Procedure

Prerequisites

• gh CLI authenticated and configured
• Remote github configured and reachable

Step-by-step

1. Fetch latest remote refs

git fetch github --prune

2. List open PRs with status

gh pr list --state open --json number,title,headRefName,statusCheckRollup,reviewDecision,mergeable,updatedAt --limit 30

Key fields:

• statusCheckRollup: array of check results — look for conclusion: "FAILURE" or conclusion: "ACTION_REQUIRED"
• reviewDecision: APPROVED, CHANGES_REQUESTED, REVIEW_REQUIRED, or empty
• mergeable: MERGEABLE, CONFLICTING, or UNKNOWN

3. Classify each PR

Build a health table:

Priority mapping:

• P0: CI failed + changes requested (both blocking)
• P1: CI failed only
• P2: Changes requested only (CI passing)
• P3: Merge conflict only
• P4: Healthy (no action needed)

4. Deep-dive for failing PRs

For each P0/P1 PR, gather failure details:

gh pr checks <number>

If checks show failure, try to get the log:

gh run view <run-id> --log-failed 2>/dev/null | tail -100

If GitHub Actions logs are not publicly accessible, reproduce locally instead (see fix-workflow.md).

5. Check for review comments

gh pr view <number> --json reviews,comments --jq '.reviews[] | select(.state == "CHANGES_REQUESTED") | .body'

Also check inline review comments:

gh api repos/{owner}/{repo}/pulls/<number>/comments --jq '.[] | {path: .path, line: .line, body: .body}'

6. Present discovery summary

Output a structured summary to the user:

## Open PR Health Check — <date>

Total open PRs: N
- 🔴 CI Failed: X
- 🟡 Changes Requested: Y  
- ⚪ Merge Conflict: Z
- 🟢 Healthy: W

### PRs requiring action (sorted by priority):
1. PR #NNN — <title> — CI failed (build error) + changes requested
2. PR #NNN — <title> — CI failed (test failure)
...

Wait for user confirmation before proceeding to fix phase.

Common Fix Recipes

Patterns observed in this project that frequently cause CI failures or review feedback.

---

Recipe 1: debug() type mismatch

Symptom:

TS2345: Argument of type 'string' is not assignable to parameter of type 'Error | object'

Root cause: The debug(message, data?) helper expects the second argument to be Error | object, but code passes error.message (a string).

Fix:

// ❌ Wrong
debug('operation skipped:', error instanceof Error ? error.message : String(error));

// ✅ Correct
debug('operation skipped:', error instanceof Error ? error : new Error(String(error)));

---

Recipe 2: Block-scoped variable referenced outside

Symptom:

ReferenceError: command is not defined

Root cause: Variable declared with const inside if/else block, but referenced after the block.

Fix: Move the declaration before the if/else:

// ❌ Wrong
if (condition) {
  const value = computeA();
  doWork(value);
} else {
  const value = computeB();
  doWork(value);
}
return { result: value }; // ReferenceError!

// ✅ Correct
let value: string;
if (condition) {
  value = computeA();
  doWork(value);
} else {
  value = computeB();
  doWork(value);
}
return { result: value };

Or, if the value is identical in both branches, extract it:

const value = computeShared();
if (condition) {
  doWorkA(value);
} else {
  doWorkB(value);
}
return { result: value };

---

Recipe 3: Duplicate function after refactoring

Symptom: Module-level function exists, but an inline copy was left behind inside another function.

Root cause: Refactoring moved a helper to module scope but forgot to delete the old inline definition.

Fix: Delete the inline copy; keep only the module-level definition.

// Module level (keep this)
function toJSONString(v: any) { ... }

// Inside another function (delete this)
const toJSONString = (v: any) => ...;  // ← remove

---

Recipe 4: Path validation edge case

Symptom: validateAndNormalizePath rejects valid paths when cwd is the filesystem root /.

Root cause: cwd + path.sep produces // when cwd is /, and normalizedPath.startsWith('//') fails.

Fix:

// ❌ Wrong
if (!normalizedPath.startsWith(cwd + path.sep) && normalizedPath !== cwd) {

// ✅ Correct
const prefix = cwd.endsWith(path.sep) ? cwd : cwd + path.sep;
if (!normalizedPath.startsWith(prefix) && normalizedPath !== cwd) {

---

Recipe 5: Test environment dependency

Symptom: Tests fail in CI because cloud credentials are not available.

Root cause: Test directly calls cloud API without checking environment.

Fix: Use test.skipIf pattern:

// ❌ Wrong
test('should query database', async () => {
  const result = await cloudbase.query(...);
  expect(result).toBeDefined();
});

// ✅ Correct
test.skipIf(!process.env.CLOUDBASE_ENV_ID)(
  'should query database',
  async () => {
    const result = await cloudbase.query(...);
    expect(result).toBeDefined();
  }
);

---

Recipe 6: Security review — input validation

Common review feedback: "User input should be validated before use."

Fix patterns:

// Whitelist validation for dynamic identifiers
const ALLOWED = new Set(['id', 'name', 'createdAt']);
if (!ALLOWED.has(input.orderBy)) {
  throw new Error(`Invalid orderBy: ${input.orderBy}`);
}

// Path traversal prevention
const normalized = path.resolve(inputPath);
if (!normalized.startsWith(allowedBase + path.sep)) {
  throw new Error('Path traversal detected');
}

// Command injection prevention
// Never interpolate user input into shell commands
// Use spawn with argument arrays instead of exec with string

---

Recipe 7: Merge conflict resolution

Symptom: PR shows "This branch has conflicts that must be resolved."

Fix:

git checkout <pr-branch>
git fetch github main
git merge github/main
# Resolve conflicts in editor
git add <resolved-files>
git commit -m 'chore: 🔀 resolve merge conflicts with main'
git push github <pr-branch>

Rules:

• Prefer merge over rebase for PR branches (preserves history).
• After resolving, always rebuild and retest.

PR Fix Workflow

Pre-flight

1. Note the current branch name:

   git branch --show-current

2. Stash any uncommitted changes:

   git stash

3. Fetch latest:

   git fetch github

Fix loop (per PR)

Step 1 — Switch to PR branch

git checkout -B <branch-name> github/<branch-name>

Step 2 — Reproduce locally

Run the same pipeline as CI:

cd mcp
npm ci           # clean install (match CI)
npm run build    # webpack build
npm run test     # vitest

Capture the exact error output — this is your fix target.

Step 3 — Analyze root cause

Common patterns in this project:

Step 4 — Apply fix

• Keep changes minimal — fix only what's broken.
• Don't mix refactoring with bug fixes.
• If multiple issues exist, fix them all in one commit (per PR).

Step 5 — Verify locally

cd mcp
npm run build    # must pass cleanly
npm run test     # all tests must pass

If build or test fails, go back to Step 3.

Step 6 — Commit

Use conventional-changelog format with emoji:

git add <changed-files>
git commit -m 'fix(<scope>): 🔧 <english description of what was fixed>'

Scope examples:

• cloudrun — changes to cloudrun.ts
• security — security-related fixes
• code-quality — type fixes, dead code removal
• database — database-related changes
• test — test fixes

Step 7 — Push

git push github <branch-name>

Never force-push unless explicitly asked.

Step 8 — Return to original branch

git checkout <original-branch>
git stash pop   # only if we stashed in pre-flight

Multi-PR session

When fixing multiple PRs in one session:

1. Complete the full fix loop for PR A before starting PR B. 2. Track progress with a checklist. 3. Only stash once at the beginning; pop once at the end. 4. If fixing PR B requires changes that conflict with PR A's branch, note it and proceed carefully.

Post-fix verification

After pushing all fixes:

# Check CI status for each fixed PR
gh pr checks <number>

Wait ~2-3 minutes for CI to pick up the new commit.

If CI still fails: 1. Re-read the failure 2. Determine if it's the same issue or a new one 3. Loop back to Step 2

Completion summary template

## PR Fix Session — <date>

### Fixed:
- ✅ PR #NNN (<branch>) — <what was fixed> — commit <hash>
- ✅ PR #NNN (<branch>) — <what was fixed> — commit <hash>

### Still needs attention:
- ⚠️ PR #NNN — <reason>

### CI verification:
- PR #NNN: ✅ passing
- PR #NNN: ⏳ waiting for CI

PR Triage Rules

Priority matrix

Within the same priority, sort by

1. Recency — older PRs first (they've been blocking longer) 2. Scope — smaller PRs first (quicker wins, unblock faster) 3. Author — PRs from the same author grouped together (context locality)

CI failure categories

Build errors (most common)

Symptoms:

• TypeScript compilation errors (TS2345, TS2322, etc.)
• Webpack bundling failures
• Missing module or import errors

Quick check:

cd mcp && npm run build 2>&1 | grep -E "error TS|ERROR in|Module not found"

Test failures

Symptoms:

• Vitest assertion failures
• Timeout errors
• Missing environment variables (tests skipped vs failed)

Quick check:

cd mcp && npm run test 2>&1 | grep -E "FAIL|AssertionError|Timeout"

Lint / type-check errors

Symptoms:

• ESLint rule violations
• Prettier format mismatches

Quick check:

cd mcp && npx tsc --noEmit 2>&1 | head -30

Review feedback categories

Style / formatting

• Variable naming, code style preferences
• Low risk, quick fix

Logic / correctness

• Bug in the implementation, wrong behavior
• Medium risk, requires careful fix + test

Security

• Input validation, injection risks, auth issues
• High risk, must verify fix thoroughly

Architecture / design

• Approach disagreement, refactoring suggestions
• May require discussion before fixing — present options to user

Decision framework

For each PR, ask:

1. Can I reproduce the failure locally?

• Yes → proceed to fix
• No → investigate CI environment differences

2. Is the fix straightforward?

• Yes → fix directly
• No → present analysis and options to user

3. Does the fix touch shared code?

• Yes → extra caution, verify no regression
• No → safe to proceed

4. Is there a review disagreement (architecture-level)?

• Yes → present both sides to user, don't auto-fix
• No → implement the requested change