Repository navigation
[#300]: Stop auto-closing issues; hand off to In QA - #301
Conversation
Prefer Refs #NNN over closing keywords so merge leaves tickets open for QA review on Project 4.
|
Warning Review limit reached
Next review available in: 54 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR changes issue-linking guidance from closing references to ChangesIssue reference and QA workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant resolveLinkedIssueNumbers
participant notifyLinkedIssues
participant ProjectBoard
PullRequest->>resolveLinkedIssueNumbers: title and body references
resolveLinkedIssueNumbers-->>notifyLinkedIssues: resolved issue numbers
notifyLinkedIssues->>ProjectBoard: add preview comment and move eligible issues to In QA
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/issue-tracking.md`:
- Around line 69-80: Update resolveLinkedIssueNumbers in
.github/scripts/preview-notify-linked-issues.js to recognize Refs `#NNN` while
preserving PR-number exclusion, and keep Part of `#NNN` separate so it does not
trigger In QA; add regression coverage for linked preview comments, In QA
transitions, exact Details-line formatting, and positive/negative Refs/Part of
cases. At docs/issue-tracking.md:69-80, docs/issue-tracking.md:33-33,
.cursor/commands/create-pr.md:66-66, .cursor/commands/create-pr.md:87-87,
.cursor/commands/generate-pr-description.md:70-70,
.cursor/commands/generate-pr-description.md:110-110,
.cursor/rules/delivery.mdc:16-16, .cursor/rules/delivery.mdc:31-31,
.cursor/rules/docs.mdc:18-18, .cursor/templates/pr-template.md:7-15,
.github/README.md:33-33, docs/guides/qa-preview-testing.md:60-67, and
docs/guides/project-board.md:33-40, preserve the stated Refs/non-closing and
Part of contracts and update or verify documentation only as needed to match the
corrected resolver; each site requires coverage or validation of the documented
workflow rather than changing the contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: acc7a9da-2374-461e-98e0-d62798c50294
📒 Files selected for processing (10)
.cursor/commands/create-pr.md.cursor/commands/generate-pr-description.md.cursor/rules/delivery.mdc.cursor/rules/docs.mdc.cursor/rules/project-board.mdc.cursor/templates/pr-template.md.github/README.mddocs/guides/project-board.mddocs/guides/qa-preview-testing.mddocs/issue-tracking.md
| - Under Details: `Refs #NNN` on its own line (links the issue without closing it) | ||
| - Do **not** use GitHub closing keywords (`Closes`, `Fixes`, `Resolves`) — merged PRs must not auto-close issues | ||
| - For related / stacked work that is not the full ticket, say “Part of #NNN” in prose, or link manually in the PR sidebar | ||
| - After opening a PR, set Project 4 Status to **`In PR Review`** (if not already) | ||
| - After merge (or when ready for QA), set Project 4 Status to **`In QA`** — leave the GitHub issue **open** | ||
| - **Template source of truth:** [`.cursor/templates/pr-template.md`](../.cursor/templates/pr-template.md) — also required by [delivery.mdc](../.cursor/rules/delivery.mdc); generate with `/generate-pr-description` or `/create-pr` | ||
|
|
||
| ### Closing keywords | ||
| ### Linking without auto-close | ||
|
|
||
| Use [GitHub closing keywords](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) (`Closes`, `Fixes`, `Resolves`) when the PR finishes the issue. | ||
| Use `Refs #NNN` (or sidebar linking) so GitHub [does not auto-close](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) the issue on merge. | ||
|
|
||
| `Refs #NNN` is **not** a GitHub closing keyword — it will not auto-close. Prefer `Closes #NNN` for completed work. After merge, confirm Project 4 Status is **`Done`**. | ||
| Never use `Closes`, `Fixes`, or `Resolves` in PR bodies for ticketed work. Issues stay open until QA finishes; humans set **`Done`** (and may close the issue) after **Passed QA** / release process — not on merge alone. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Update the preview issue resolver before enforcing Refs #NNN``.
The reviewed files now require Refs #NNN``, but .github/scripts/preview-notify-linked-issues.js still parses only `Closes`, `Fixes`, `Resolves`, and `closingIssuesReferences`. A compliant PR therefore produces no linked issue number. The preview workflow can skip the linked-issue comment and the Project 4 `In QA` transition.
Update resolveLinkedIssueNumbers to parse Refs #NNN, keep the PR-number exclusion, and add tests for the preview comment and `In QA` paths. Treat `Part of `#NNN separately; do not move a parent ticket to In QA unless that behavior is intentional.
docs/issue-tracking.md#L69-L80: Keep the non-closing contract and update the resolver before rollout.docs/issue-tracking.md#L33-L33: Keep the issue-creation guidance and add resolver coverage..cursor/commands/create-pr.md#L66-L66: Keep generatedRefs#NNN`` references and test their discovery..cursor/commands/create-pr.md#L87-L87: Keep the Details-line requirement and test that exact format..cursor/commands/generate-pr-description.md#L70-L70: Keep the lookup guidance and verify resolver support..cursor/commands/generate-pr-description.md#L110-L110: Keep the ticket pattern and add a regression test..cursor/rules/delivery.mdc#L16-L16: Keep the delivery rule and update the downstream consumer..cursor/rules/delivery.mdc#L31-L31: Keep the template contract and test generated PR bodies..cursor/rules/docs.mdc#L18-L18: Keep the pointer only when the referenced workflow supportsRefs..cursor/templates/pr-template.md#L7-L15: Keep the checklist and placeholder, with parser coverage..github/README.md#L33-L33: Keep the preview requirement and align it with issue discovery.docs/guides/qa-preview-testing.md#L60-L67: Test the no-comment andIn QAtroubleshooting path withRefs.docs/guides/project-board.md#L33-L40: Preserve theRefs/Part ofdistinction and add positive and negative tests.
As per coding guidelines, verify documentation claims against the implementation before enforcing the new reference contract.
📍 Affects 9 files
docs/issue-tracking.md#L69-L80(this comment)docs/issue-tracking.md#L33-L33.cursor/commands/create-pr.md#L66-L66.cursor/commands/create-pr.md#L87-L87.cursor/commands/generate-pr-description.md#L70-L70.cursor/commands/generate-pr-description.md#L110-L110.cursor/rules/delivery.mdc#L16-L16.cursor/rules/delivery.mdc#L31-L31.cursor/rules/docs.mdc#L18-L18.cursor/templates/pr-template.md#L7-L15.github/README.md#L33-L33docs/guides/qa-preview-testing.md#L60-L67docs/guides/project-board.md#L33-L40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/issue-tracking.md` around lines 69 - 80, Update
resolveLinkedIssueNumbers in .github/scripts/preview-notify-linked-issues.js to
recognize Refs `#NNN` while preserving PR-number exclusion, and keep Part of `#NNN`
separate so it does not trigger In QA; add regression coverage for linked
preview comments, In QA transitions, exact Details-line formatting, and
positive/negative Refs/Part of cases. At docs/issue-tracking.md:69-80,
docs/issue-tracking.md:33-33, .cursor/commands/create-pr.md:66-66,
.cursor/commands/create-pr.md:87-87,
.cursor/commands/generate-pr-description.md:70-70,
.cursor/commands/generate-pr-description.md:110-110,
.cursor/rules/delivery.mdc:16-16, .cursor/rules/delivery.mdc:31-31,
.cursor/rules/docs.mdc:18-18, .cursor/templates/pr-template.md:7-15,
.github/README.md:33-33, docs/guides/qa-preview-testing.md:60-67, and
docs/guides/project-board.md:33-40, preserve the stated Refs/non-closing and
Part of contracts and update or verify documentation only as needed to match the
corrected resolver; each site requires coverage or validation of the documented
workflow rather than changing the contract.
Source: Coding guidelines
Update preview-notify to link via Refs (not Part of), with regression tests and docs aligned to the resolver.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/README.md:
- Around line 33-34: Clarify the preview-ready PR guidance in the workflow step:
require Refs `#NNN` only for full-ticket QA handoffs, and explicitly state that
stacked or partial PRs using Part of `#NNN` must not receive the preview-build
label or move the linked issue to In QA.
In @.github/scripts/preview-notify-linked-issues.js:
- Around line 92-93: Update LINKED_ISSUE_RE to require word boundaries around
the linked-issue action keywords so substrings such as “unresolved” do not match
“resolved”; preserve valid close, fix, resolve, and refs forms. Add a regression
test covering “unresolved `#42`” and verify notifyLinkedIssues does not process
issue 42 for that input.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a4339aa4-57c4-42cd-9389-88d60b4a7082
📒 Files selected for processing (7)
.github/README.md.github/scripts/preview-notify-linked-issues.js.github/scripts/preview-notify-linked-issues.test.jsdocs/ci.mddocs/guides/project-board.mddocs/guides/qa-preview-testing.mddocs/issue-tracking.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/guides/project-board.md
- docs/guides/qa-preview-testing.md
| 2. Add the label to a PR when ready for QA (PR should reference its ticket with `Refs #NNN` on its own Details line — not `Part of #NNN`). | ||
| 3. Workflow starts a **fresh Android preview APK** (binary only — no OTA / `eas update`), posts the same install comment on the **PR** and each **linked GitHub issue** (`Refs` / legacy closing keywords via [`.github/scripts/preview-notify-linked-issues.js`](scripts/preview-notify-linked-issues.js)), and best-effort moves Project 4 cards from `In PR Review` / `In Progress (Dev)` → **`In QA`**. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clarify the Refs rule for partial PRs.
docs/issue-tracking.md allows Part of #NNN for stacked or partial work. It also states that the preview resolver ignores Part-of-only references. This step says that a preview-ready PR must use `Refs `#NNN instead. A partial PR could therefore link its parent issue and move it to In QA before the full ticket is ready.
Limit Refs #NNN`` to full-ticket QA handoffs, or state that stacked and partial PRs must not use the preview-build label.
Proposed clarification
-2. Add the label to a PR when ready for QA (PR should reference its ticket with `Refs `#NNN`` on its own Details line — not `Part of `#NNN``).
+2. Add the label to a PR for a full-ticket QA handoff. Use `Refs `#NNN`` on its own Details line.
+ For stacked or partial PRs, use `Part of `#NNN`` and do not expect automatic issue QA handling.📝 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.
| 2. Add the label to a PR when ready for QA (PR should reference its ticket with `Refs #NNN` on its own Details line — not `Part of #NNN`). | |
| 3. Workflow starts a **fresh Android preview APK** (binary only — no OTA / `eas update`), posts the same install comment on the **PR** and each **linked GitHub issue** (`Refs` / legacy closing keywords via [`.github/scripts/preview-notify-linked-issues.js`](scripts/preview-notify-linked-issues.js)), and best-effort moves Project 4 cards from `In PR Review` / `In Progress (Dev)` → **`In QA`**. | |
| 2. Add the label to a PR for a full-ticket QA handoff. Use `Refs `#NNN`` on its own Details line. | |
| For stacked or partial PRs, use `Part of `#NNN`` and do not expect automatic issue QA handling. | |
| 3. Workflow starts a **fresh Android preview APK** (binary only — no OTA / `eas update`), posts the same install comment on the **PR** and each **linked GitHub issue** (`Refs` / legacy closing keywords via [`.github/scripts/preview-notify-linked-issues.js`](scripts/preview-notify-linked-issues.js)), and best-effort moves Project 4 cards from `In PR Review` / `In Progress (Dev)` → **`In QA`**. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: The official name of this software platform is spelled with a capital “H”.
Context: ...* (Refs / legacy closing keywords via [`.github/scripts/preview-notify-linked-issues.js...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/README.md around lines 33 - 34, Clarify the preview-ready PR
guidance in the workflow step: require Refs `#NNN` only for full-ticket QA
handoffs, and explicitly state that stacked or partial PRs using Part of `#NNN`
must not receive the preview-build label or move the linked issue to In QA.
| const LINKED_ISSUE_RE = | ||
| /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs)\s*:?\s*#(\d+)/gi; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add token boundaries to LINKED_ISSUE_RE.
unresolved #42 matches `resolved `#42 because the expression does not require a word boundary. The resolver then returns 42, and notifyLinkedIssues can comment on and move that unrelated Project 4 card to In QA. Match complete keywords only. Add a regression test for this input.
Proposed fix
- /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs)\s*:?\s*#(\d+)/gi;
+ /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs)\b\s*:?\s*#(\d+)/gi;+ it('does not match a keyword inside ordinary prose', () => {
+ expect(
+ collectLinkedIssueNumbersFromText(
+ 'Title',
+ 'The dependency remains unresolved `#42`.',
+ 99,
+ ),
+ ).toEqual([]);
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/preview-notify-linked-issues.js around lines 92 - 93, Update
LINKED_ISSUE_RE to require word boundaries around the linked-issue action
keywords so substrings such as “unresolved” do not match “resolved”; preserve
valid close, fix, resolve, and refs forms. Add a regression test covering
“unresolved `#42`” and verify notifyLinkedIssues does not process issue 42 for
that input.
Root package.json is type module; .js files break require() in the preview-build workflow.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/preview-notify-linked-issues.cjs (1)
74-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMove Project 4 cards only after merge.
notifyLinkedIssuesmoves a card toIn QAimmediately after a preview comment. The test confirms this behavior for a PR payload with no merged state. This can move anIn PR Reviewcard toIn QAbefore the PR merges.
.github/scripts/preview-notify-linked-issues.cjs#L74-L82: GatemoveIssuesToInQaon a confirmed merged PR, or invoke it only from a post-merge workflow..github/scripts/preview-notify-linked-issues.test.cjs#L194-L264: Setpull_request.merged: truein the positive case. Add a case that confirms an unmerged PR does not move the card.Based on learnings, set the assigned ticket to
In QAafter merge or when ready for QA.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/preview-notify-linked-issues.cjs around lines 74 - 82, Gate moveIssuesToInQa in notifyLinkedIssues on pull_request.merged being true so cards move to In QA only after merge; update .github/scripts/preview-notify-linked-issues.cjs lines 74-82. In .github/scripts/preview-notify-linked-issues.test.cjs lines 194-264, mark the positive payload as merged and add coverage confirming an unmerged pull request does not move the card.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/preview-notify-linked-issues.test.cjs:
- Line 1: Update the require call in the test module to load
preview-notify-linked-issues.cjs, matching the tracked implementation and
workflow-resolved path instead of the non-existent .js file.
---
Outside diff comments:
In @.github/scripts/preview-notify-linked-issues.cjs:
- Around line 74-82: Gate moveIssuesToInQa in notifyLinkedIssues on
pull_request.merged being true so cards move to In QA only after merge; update
.github/scripts/preview-notify-linked-issues.cjs lines 74-82. In
.github/scripts/preview-notify-linked-issues.test.cjs lines 194-264, mark the
positive payload as merged and add coverage confirming an unmerged pull request
does not move the card.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 433e3cac-f63e-4e9f-860e-1bb3826cb94e
📒 Files selected for processing (2)
.github/scripts/preview-notify-linked-issues.cjs.github/scripts/preview-notify-linked-issues.test.cjs
Update the workflow require path, test import, and doc links to the renamed script, and add a .cjs testMatch entry so the resolver tests are discovered.
…auto-closing-issues-qa
TLDR
Stop using GitHub closing keywords (
Closes/Fixes/Resolves) so merged PRs no longer auto-close issues. Ticketed work links withRefs #NNNand moves to In QA on Project 4 for QA review before Done.Reviewer checklist
Refs #NNN— do not useCloses/Fixes/Resolves)AGENTS.md)Details
Refs #300
Update agent delivery docs, PR template, slash commands, and QA/preview docs so issues stay open after merge and board Status goes to In QA instead of auto-close / Done-on-merge.
Type of change:
Technical changes
.cursor/templates/pr-template.md—Refs #NNN; ban closing keywords.cursor/rules/delivery.mdc— link withRefs; post-merge → In QA.cursor/rules/project-board.mdc— Done only after QA, not on merge.cursor/rules/docs.mdc— pointer text.cursor/commands/create-pr.md/generate-pr-description.md—Refs #NNNdocs/issue-tracking.md— linking-without-auto-close policydocs/guides/project-board.md— In QA handoff; note Require Closes #NNN only on PRs targeting main (stacked-PR tracker guardrail) #274 superseded by Stop auto-closing issues on PR merge; hand off to In QA #300docs/guides/qa-preview-testing.md/.github/README.md—Refsfor ticket linksTesting
Docs-only. App gates (
format:check,lint,typecheck,npm test -- --ci) not required for this change.How to verify
.cursor/templates/pr-template.mdand confirm Details usesRefs #NNN(notCloses).Refs #is instructed; closing keywords are forbidden for ticketed PRs.docs/issue-tracking.mdanddocs/guides/project-board.mdsay post-merge Status is In QA, issue stays open.Expected: New PRs link issues without auto-close; merge hands tickets to QA on the board.
Follow-ups
AddProjectV2ItemByIdpermission)Summary by CodeRabbit
Documentation
Refs#NNN`` references.Bug Fixes