Skip to content

[github] bug: review features with merged PRs not auto-transitioned to done - #4122

Closed
mabry1985 wants to merge 1 commit into
mainfrom
feature/github-bug-review-features-with-merged-prs-not-zw4h685
Closed

mabry1985 wants to merge 1 commit into
mainfrom
feature/github-bug-review-features-with-merged-prs-not-zw4h685

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Summary

SPARC PRD: Auto-transition review features to done when linked PR merges

Situation

The protoMaker feature lifecycle manages features through statuses: backlog → in_progress → review → done. When a PR merges, features should automatically transition to done via three reconciliation paths:

  1. Branch match (feature-scheduler.ts:862-884) — matches feature.branchName against merged PR headRefName
  2. prMergedAt guard (feature-scheduler.ts:893-914) — catches features whe...

Closes #4108


Created automatically by Automaker

@protoquinn

protoquinn Bot commented Jun 6, 2026

Copy link
Copy Markdown

👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@mabry1985, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 19 minutes and 9 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f313d6cd-ecc8-4b79-83e0-fa09c3f9369f

📥 Commits

Reviewing files that changed from the base of the PR and between 07cf379 and 44ccec3.

📒 Files selected for processing (2)
  • apps/server/src/routes/webhooks/routes/github.ts
  • apps/server/tests/unit/routes/github-webhook-pr-number-fallback.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/github-bug-review-features-with-merged-prs-not-zw4h685

Comment @coderabbitai help to get the list of available commands and usage tips.

@protoquinn

protoquinn Bot commented Jun 6, 2026

Copy link
Copy Markdown

CI is still in_progress. Submitting a non-blocking COMMENT review with my diff-based assessment now — formal verdict (PASS/FAIL) will come once CI is terminal.


PR #4122 | [github] bug: review features with merged PRs not auto-transitioned to done

VERDICT: WARN (CI not yet terminal — see below)


CI Status

  • checks: in_progress
  • test: in_progress
  • build: in_progress
  • Close external code contributions: skipped

Diff Review

The PR adds a PR-number fallback to the GitHub merged-PR webhook handler (github.ts). When findFeatureByBranch() returns null, the handler now scans all configured projects via findByPRNumber() to find the feature — covering board-only features where prNumber was linked but branchName was not. A new 297-line test suite (github-webhook-pr-number-fallback.test.ts) covers both the new fallback path and the existing branch-name path. Closes #4108.

Observations

  • MEDIUM (github.ts:646–649): When the fallback locates a feature by PR number, a synthetic { featureId, title } object is passed to the downstream feature-transition logic instead of the full loaded feature. The caller findFeatureByBranch() is invoked with this synthetic object — verify that transitionFeatureFromGitHubEvent() does not require additional fields (e.g. status, branchName) that would be absent here. If it does, the fallback branch will silently degrade.
    • Recommended fix: Add an integration test covering the fallback path end-to-end (verify status transitions to done) to catch any missing-field regression.
  • Gap: Clawpatch structural review was unavailable (checkout cache returned invalid SHA for main) — cross-file interaction between the fallback and the transition logic could not be verified.
  • LOW: let projectPath / let feature are used for reassignment — appropriate here, but worth a comment explaining the two-phase lookup pattern for future maintainers.

No unresolved CodeRabbit threads.


Once CI reaches a terminal state, I will re-inspect and issue the formal APPROVED or CHANGES_REQUESTED verdict.

— Quinn, QA Engineer

@mabry1985

Copy link
Copy Markdown
Member Author

Closing as superseded — #4119 (same feature: merged-PR→done auto-transition) already MERGED, and this larger re-attempt has test:FAILURE. The board feature is already done. Reopen the feature if #4119's fix proves incomplete. Fleet triage.

@mabry1985 mabry1985 closed this Jun 7, 2026
@mabry1985
mabry1985 deleted the feature/github-bug-review-features-with-merged-prs-not-zw4h685 branch June 7, 2026 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: review features with merged PRs not auto-transitioned to done

1 participant