From eb1ec4553aab9824370ad213b854ac259e959027 Mon Sep 17 00:00:00 2001 From: mattrace-gloo Date: Tue, 4 Aug 2026 14:57:06 -0700 Subject: [PATCH 1/4] [#300]: Stop auto-closing issues; hand off to In QA Prefer Refs #NNN over closing keywords so merge leaves tickets open for QA review on Project 4. --- .cursor/commands/create-pr.md | 4 ++-- .cursor/commands/generate-pr-description.md | 4 ++-- .cursor/rules/delivery.mdc | 6 +++--- .cursor/rules/docs.mdc | 2 +- .cursor/rules/project-board.mdc | 2 +- .cursor/templates/pr-template.md | 4 ++-- .github/README.md | 2 +- docs/guides/project-board.md | 9 +++++---- docs/guides/qa-preview-testing.md | 4 ++-- docs/issue-tracking.md | 16 +++++++++------- 10 files changed, 28 insertions(+), 25 deletions(-) diff --git a/.cursor/commands/create-pr.md b/.cursor/commands/create-pr.md index 9a3fcec1..6935251a 100644 --- a/.cursor/commands/create-pr.md +++ b/.cursor/commands/create-pr.md @@ -63,7 +63,7 @@ Pre-fill: **From GitHub issue:** -- TLDR, Details, `Closes #NNN` when the PR completes the issue +- TLDR, Details, `Refs #NNN` (never `Closes` / `Fixes` / `Resolves`) **From branch analysis:** @@ -84,7 +84,7 @@ Keep output under ~400 lines; no nested fenced code blocks inside the PR body. - Detect issue number from branch: `…/173-…` or leading `173-…` - Fetch with `gh issue view NNN --repo eten-tech-foundation/fluent-mobile` -- Body must include `Closes #NNN` on its own line under Details when the PR completes the issue +- Body must include `Refs #NNN` on its own line under Details (non-closing — issues stay open for QA) ## GitHub (`gh`) diff --git a/.cursor/commands/generate-pr-description.md b/.cursor/commands/generate-pr-description.md index 34a8bb15..46ae285f 100644 --- a/.cursor/commands/generate-pr-description.md +++ b/.cursor/commands/generate-pr-description.md @@ -67,7 +67,7 @@ gh issue view --repo eten-tech-foundation/fluent-mobile --json title,body 1. Load template from `.cursor/templates/pr-template.md` 2. **Synthesize PR title** (mandatory — see above) 3. Detect branch type: `feature/`, `fix/`, `chore/` → suggest change type checkbox -4. Fetch GitHub issue when issue number present (`Closes #NNN` in Details) +4. Fetch GitHub issue when issue number present (`Refs #NNN` in Details) 5. Pre-fill Technical Changes and Testing from diff + [docs/AGENT_ONBOARDING.md](../../docs/AGENT_ONBOARDING.md) gates ## Output format @@ -107,7 +107,7 @@ gh issue view --repo eten-tech-foundation/fluent-mobile --json title,body ## Ticket patterns -- GitHub issues: `[#123]` / `#123` in PR title; `Closes #123` under Details when the PR completes the issue +- GitHub issues: `[#123]` / `#123` in PR title; `Refs #123` under Details (never closing keywords) ## Quality checklist (pre-fill reminders) diff --git a/.cursor/rules/delivery.mdc b/.cursor/rules/delivery.mdc index 5b47bbe7..e6e2cd35 100644 --- a/.cursor/rules/delivery.mdc +++ b/.cursor/rules/delivery.mdc @@ -13,8 +13,8 @@ Every change — including infra, CI, docs, Dependabot follow-ups, and `expo ins 1. **Ticket on Project 4** — GitHub Issue `#NNN` **on** the [Fluent Mobile Board](https://github.com/orgs/eten-tech-foundation/projects/4/views/9) (org project #4, view 9). Ask if missing. See [docs/issue-tracking.md](../../docs/issue-tracking.md). The repo Issues list is not the triage board. (Not Linear.) Board Status/Priority edits: [docs/guides/project-board.md](../../docs/guides/project-board.md) and [project-board.mdc](./project-board.mdc) — do not sweep Product or `Dev Ready` lanes without named approval. 2. **Feature branch** — `{author}/{type}/{issue-number}-{slug}` (see `/create-pr-branch`) -3. **Pull request** — title `[#NNN]: Description`; body filled from the PR template (below); include `Closes #NNN` under Details when the PR completes the issue; set Project 4 Status to **In PR Review** -4. **Human merge** — agents open PRs and push **feature branches only**; never merge on GitHub unless the user explicitly asks for a Dependabot squash merge in that session +3. **Pull request** — title `[#NNN]: Description`; body filled from the PR template (below); include `Refs #NNN` under Details (never `Closes` / `Fixes` / `Resolves`); set Project 4 Status to **In PR Review** +4. **Human merge** — agents open PRs and push **feature branches only**; never merge on GitHub unless the user explicitly asks for a Dependabot squash merge in that session. After merge, set Project 4 Status to **In QA** (issues stay open for QA — do not auto-close) Allowed on `main` locally: `git checkout main`, `git pull`, read-only validation after merges. **Not allowed:** `git push origin main`, commits on `main`, or opening PRs with `main` as the head branch. @@ -28,7 +28,7 @@ Do **not** substitute a short “Summary / Test plan” or ad-hoc bullet list. L | ------- | ---------------- | | **TLDR** | 2–4 sentences: what / why / impact | | **Reviewer checklist** | Leave items unchecked unless verified | -| **Details** | `Closes #NNN` on its own line when the PR completes the issue; short summary; type-of-change checkboxes | +| **Details** | `Refs #NNN` on its own line (non-closing link); short summary; type-of-change checkboxes | | **Technical changes** | Key files as `` `path` `` bullets | | **Testing** | What gates ran (`format:check`, `lint`, `typecheck`, `npm test -- --ci`) or waiver | | **How to verify** | Numbered steps + **Expected** | diff --git a/.cursor/rules/docs.mdc b/.cursor/rules/docs.mdc index c0197952..5adbce04 100644 --- a/.cursor/rules/docs.mdc +++ b/.cursor/rules/docs.mdc @@ -15,7 +15,7 @@ alwaysApply: false ## Issue tracking & CI -- [docs/issue-tracking.md](../../docs/issue-tracking.md) — Project 4 Fluent Mobile Board, branch naming, PR linking (`Closes #NNN`) +- [docs/issue-tracking.md](../../docs/issue-tracking.md) — Project 4 Fluent Mobile Board, branch naming, PR linking (`Refs #NNN`; never auto-close) - [docs/guides/project-board.md](../../docs/guides/project-board.md) — board mutation hard rules (agents) - [docs/guides/expo-first-dependencies.md](../../docs/guides/expo-first-dependencies.md) — Expo-first package preference - [docs/ci.md](../../docs/ci.md) — workflow inventory + future required-check guardrail diff --git a/.cursor/rules/project-board.mdc b/.cursor/rules/project-board.mdc index 29de08b6..3abeac2b 100644 --- a/.cursor/rules/project-board.mdc +++ b/.cursor/rules/project-board.mdc @@ -13,6 +13,6 @@ Canonical policy: [docs/guides/project-board.md](../../docs/guides/project-board - **Do not** edit Status or Priority on `In Progress (Product)` (or other Product-owned columns) unless the tech lead **explicitly lists** those issue numbers. - **No board sweeps** without a proposed change set + explicit approval. Ambiguous = no. - **Do not merge PRs** unless explicitly asked (Dependabot process is the standing exception when requested). -- For the **assigned** ticket only: add to Project 4 if missing; set `In PR Review` when the PR opens; set `Done` **only after merge** (or with completion evidence on `main`) — never before. +- For the **assigned** ticket only: add to Project 4 if missing; set `In PR Review` when the PR opens; set **`In QA`** after merge (or when ready for QA) — never auto-close the issue; do **not** set `Done` until QA completes (human/QA process). Tracker overview: [docs/issue-tracking.md](../../docs/issue-tracking.md). Delivery: [delivery.mdc](./delivery.mdc). diff --git a/.cursor/templates/pr-template.md b/.cursor/templates/pr-template.md index ed9b0e94..347ec8ba 100644 --- a/.cursor/templates/pr-template.md +++ b/.cursor/templates/pr-template.md @@ -4,7 +4,7 @@ ### Reviewer checklist -- [ ] GitHub issue linked in Details (`Closes #NNN` when this PR completes it) +- [ ] GitHub issue linked in Details (`Refs #NNN` — do **not** use `Closes` / `Fixes` / `Resolves`) - [ ] How to verify steps completed or valid waiver noted below - [ ] Acceptance criteria met, **or** unmet AC waived **in the issue** with linked follow-up (see `AGENTS.md`) - [ ] Scope limited to this issue — no adjacent tickets implemented/stubbed without approval @@ -12,7 +12,7 @@ ### Details -Closes # +Refs # diff --git a/.github/README.md b/.github/README.md index fd9ebee2..a2566c28 100644 --- a/.github/README.md +++ b/.github/README.md @@ -30,7 +30,7 @@ See [`.eas/README.md`](../.eas/README.md) for Expo GitHub app and Play Store set ## PR preview (QA) 1. Create a `preview-build` label on the repo (if missing). -2. Add the label to a PR when ready for QA (PR should reference its ticket with `Closes #NNN`). +2. Add the label to a PR when ready for QA (PR should reference its ticket with `Refs #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**, and best-effort moves Project 4 cards from `In PR Review` / `In Progress (Dev)` → **`In QA`**. **QA guide (non-technical):** [`docs/guides/qa-preview-testing.md`](../docs/guides/qa-preview-testing.md) diff --git a/docs/guides/project-board.md b/docs/guides/project-board.md index fe1b4370..98c7d375 100644 --- a/docs/guides/project-board.md +++ b/docs/guides/project-board.md @@ -13,7 +13,7 @@ This guide is for **any** coding agent (Cursor, Claude Code, Copilot, etc.). Boa | `Dev Ready` | Eng can code | Product/engineering already approved this for implementation. **Do not** move these cards to `Backlog` or `Sprint Shaping` to “sequence” or “clean up” the board. | | `In Progress (Dev)` | Assignee | Actively coding. | | `In PR Review` | Reviewers | Open PR exists (or should). Prefer this over leaving a card in `Dev Ready` once a PR is up. | -| `In QA` / `Passed QA` / `To Deploy` / `Done` | QA / release | Follow human process; agents set `Done` only when closing completed work with evidence (and usually after merge). | +| `In QA` / `Passed QA` / `To Deploy` / `Done` | QA / release | Follow human process. After merge, agents set **`In QA`** (not `Done`). Do **not** auto-close issues. Agents set `Done` only when a human/QA process asks, or when closing obsolete/duplicate work with an approved change set. | ## Hard rules @@ -30,13 +30,14 @@ When implementing the **assigned** ticket only: - Add the new issue to Project 4 if missing. - Set that issue to `In Progress (Dev)` while coding (optional) and **`In PR Review`** when the PR is open. -- Set **`Done`** after merge when the issue is completed and `Closes #NNN` did not auto-close, or when closing a completed epic with evidence. +- Set **`In QA`** after merge (or when ready for QA). Leave the GitHub issue open — never use closing keywords in the PR. +- Set **`Done`** only after QA completes (human/QA process), or when closing obsolete/duplicate work with an approved change set and evidence. Do **not** “helpfully” reorder neighboring cards. -## Closing keywords and stacked PRs +## Linking keywords and stacked PRs -GitHub auto-closes only when a PR with `Closes #NNN` merges into **`main`**. Stacked PRs whose base is a feature branch must use **`Part of #NNN`** until the final PR targets `main`. See [docs/issue-tracking.md](../issue-tracking.md) and issue #274. +Do **not** use GitHub closing keywords (`Closes`, `Fixes`, `Resolves`) — they auto-close issues on merge to `main`. Always use **`Refs #NNN`** (or “Part of #NNN” for stacked/partial work). After merge, move the card to **`In QA`**. See [docs/issue-tracking.md](../issue-tracking.md). (Historical: issue #274 required `Closes` on `main`-targeting PRs; superseded by #300.) ## Related diff --git a/docs/guides/qa-preview-testing.md b/docs/guides/qa-preview-testing.md index aeb4763c..0a9db900 100644 --- a/docs/guides/qa-preview-testing.md +++ b/docs/guides/qa-preview-testing.md @@ -57,14 +57,14 @@ If the app looks wrong after installing a different PR’s preview: | Phone offers **Expo Go** | Cancel. Install **Fluent** from the **Install Fluent** link. | | Testing the wrong PR | Install again from that PR’s (or issue’s) latest bot comment. | | Install blocked | Allow **Install unknown apps** for your browser. | -| No bot comment on PR or issue | Ask a developer to add the **`preview-build`** label (and ensure the PR has `Closes #NNN`). | +| No bot comment on PR or issue | Ask a developer to add the **`preview-build`** label (and ensure the PR has `Refs #NNN`). | | expo.dev asks me to log in | Request access to the Fluent project from your team lead. | --- ## For developers -1. Link the ticket in the PR body (`Closes #NNN`). +1. Link the ticket in the PR body (`Refs #NNN` — never `Closes` / `Fixes` / `Resolves`). 2. Add the **`preview-build`** label to the PR. 3. Workflow starts a **fresh Android preview APK** (binary only — no OTA), comments on the **PR and linked issue**, and moves the ticket to **In QA** when Status was `In PR Review` or `In Progress (Dev)`. 4. Share this guide with QA: `docs/guides/qa-preview-testing.md` diff --git a/docs/issue-tracking.md b/docs/issue-tracking.md index 245e0ba7..7ad600a5 100644 --- a/docs/issue-tracking.md +++ b/docs/issue-tracking.md @@ -16,7 +16,7 @@ The [repo Issues list](https://github.com/eten-tech-foundation/fluent-mobile/iss `Backlog` · `In Progress (Product)` · `Product Ready` · `Sprint Shaping` · `Dev Ready` · `In Progress (Dev)` · `In PR Review` · `In QA` · `Passed QA` · `To Deploy` · `Done` -For open PRs awaiting review, prefer **`In PR Review`**. For merged/completed work, set **`Done`**. +For open PRs awaiting review, prefer **`In PR Review`**. After merge (or when ready for QA), set **`In QA`**. Set **`Done`** only after QA completes — never auto-close issues on PR merge. Do **not** use org Project 7 (“Fluent Mobile App”) as the primary tracker unless the team explicitly migrates. @@ -30,7 +30,7 @@ Agents must follow [guides/project-board.md](guides/project-board.md) before cha ## Where to file work -1. Create a **GitHub Issue** in `eten-tech-foundation/fluent-mobile` (needed for `#NNN`, branch names, and `Closes #NNN`). +1. Create a **GitHub Issue** in `eten-tech-foundation/fluent-mobile` (needed for `#NNN`, branch names, and `Refs #NNN`). 2. **Add the issue to Project 4** and set Status on the Fluent Mobile Board (view 9). 3. Assign the owner on the issue. @@ -66,16 +66,18 @@ See [`.cursor/commands/create-pr-branch.md`](../.cursor/commands/create-pr-branc - **Base branch:** `main` - **Title:** `[#NNN]: Short description` (or `#NNN: Short description`) — match existing PR style in this repo - **Body:** **required** — fill [`.cursor/templates/pr-template.md`](../.cursor/templates/pr-template.md) (TLDR, Reviewer checklist, Details, Technical changes, Testing, How to verify, Follow-ups). Prefer `/generate-pr-description` or `/create-pr`. Do not ship a short Summary/Test plan substitute. - - Under Details: `Closes #NNN` when the PR **completes** the issue (auto-closes on merge to `main`) - - For related work that must stay open, say “Part of #NNN” in prose, or link manually in the PR sidebar — do not use a closing keyword + - 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. ## Agents / delivery From ce3f8d26742b6562b07ef3ebac07e2e4d758e1cf Mon Sep 17 00:00:00 2001 From: mattrace-gloo Date: Tue, 4 Aug 2026 15:24:15 -0700 Subject: [PATCH 2/4] [#300]: Recognize Refs #NNN in preview issue linking Update preview-notify to link via Refs (not Part of), with regression tests and docs aligned to the resolver. --- .github/README.md | 4 +- .../scripts/preview-notify-linked-issues.js | 85 ++++- .../preview-notify-linked-issues.test.js | 349 ++++++++++++++++++ docs/ci.md | 2 +- docs/guides/project-board.md | 2 +- docs/guides/qa-preview-testing.md | 4 +- docs/issue-tracking.md | 2 + 7 files changed, 432 insertions(+), 16 deletions(-) create mode 100644 .github/scripts/preview-notify-linked-issues.test.js diff --git a/.github/README.md b/.github/README.md index a2566c28..c5a84511 100644 --- a/.github/README.md +++ b/.github/README.md @@ -30,8 +30,8 @@ See [`.eas/README.md`](../.eas/README.md) for Expo GitHub app and Play Store set ## PR preview (QA) 1. Create a `preview-build` label on the repo (if missing). -2. Add the label to a PR when ready for QA (PR should reference its ticket with `Refs #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**, and best-effort moves Project 4 cards from `In PR Review` / `In Progress (Dev)` → **`In QA`**. +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`**. **QA guide (non-technical):** [`docs/guides/qa-preview-testing.md`](../docs/guides/qa-preview-testing.md) diff --git a/.github/scripts/preview-notify-linked-issues.js b/.github/scripts/preview-notify-linked-issues.js index 570489ef..1c461c75 100644 --- a/.github/scripts/preview-notify-linked-issues.js +++ b/.github/scripts/preview-notify-linked-issues.js @@ -4,6 +4,11 @@ * * Soft-fails: preview success must not fail if issue/board side effects fail. * + * Linked tickets are resolved from PR title/body via `Refs #NNN` (preferred) + * and legacy closing keywords (`Closes` / `Fixes` / `Resolves`). `Part of #NNN` + * is intentionally ignored so stacked/partial work does not get preview + * comments or an In QA board move. + * * Env (optional): * PROJECT_BOARD_TOKEN — PAT with org project write (preferred for Project 4) * FLUENT_PROJECT_ID — Projects V2 node id (default: Fluent Project 4) @@ -18,7 +23,7 @@ * @param {string[]} args.commentMarkers — substrings that identify prior bot preview comments * @param {typeof import('@actions/github').getOctokit} [args.getOctokit] */ -module.exports = async function notifyLinkedIssues({ +async function notifyLinkedIssues({ github, context, core, @@ -78,25 +83,75 @@ module.exports = async function notifyLinkedIssues({ } catch (error) { core.warning(`Could not update Project 4 Status: ${error.message}`); } -}; +} + +/** + * Preferred non-closing link (`Refs #NNN`) plus legacy GitHub closing keywords. + * Does not match `Part of #NNN`. + */ +const LINKED_ISSUE_RE = + /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs)\s*:?\s*#(\d+)/gi; -const CLOSING_KEYWORD_RE = - /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s*#(\d+)/gi; +/** + * Stacked / partial work — must not trigger preview ticket comments or In QA. + * Exported for regression tests only. + */ +const PART_OF_ISSUE_RE = /\bpart\s+of\s*:?\s*#(\d+)/gi; -async function resolveLinkedIssueNumbers(github, owner, repo, prNumber) { +/** + * Collect issue numbers from PR title + body text. + * Includes `Refs #NNN` and closing keywords; excludes `Part of #NNN`. + * + * @param {string} title + * @param {string | null | undefined} body + * @param {number} prNumber — excluded so the PR is never treated as a ticket + * @returns {number[]} + */ +function collectLinkedIssueNumbersFromText(title, body, prNumber) { const numbers = new Set(); + const text = `${title}\n${body || ''}`; + + for (const match of text.matchAll(LINKED_ISSUE_RE)) { + numbers.add(Number(match[1])); + } + + // Never treat the PR itself as a ticket (same number space) + numbers.delete(prNumber); + return [...numbers].sort((a, b) => a - b); +} + +/** + * True when the number appears only as `Part of #NNN` (or not as a linked + * Refs/closing keyword). Used in tests to document the Part of contract. + * + * @param {string} title + * @param {string | null | undefined} body + * @param {number} issueNumber + */ +function isPartOfOnlyReference(title, body, issueNumber) { + const text = `${title}\n${body || ''}`; + const linked = new Set(); + for (const match of text.matchAll(LINKED_ISSUE_RE)) { + linked.add(Number(match[1])); + } + if (linked.has(issueNumber)) return false; + for (const match of text.matchAll(PART_OF_ISSUE_RE)) { + if (Number(match[1]) === issueNumber) return true; + } + return false; +} + +async function resolveLinkedIssueNumbers(github, owner, repo, prNumber) { const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber, }); - for (const match of `${pr.title}\n${pr.body || ''}`.matchAll( - CLOSING_KEYWORD_RE, - )) { - numbers.add(Number(match[1])); - } + const numbers = new Set( + collectLinkedIssueNumbersFromText(pr.title, pr.body, prNumber), + ); try { const result = await github.graphql( @@ -305,3 +360,13 @@ async function findProjectItem(github, projectId, owner, repo, issueNumber) { statusName: match.fieldValueByName?.name || '', }; } + +module.exports = notifyLinkedIssues; +module.exports.collectLinkedIssueNumbersFromText = + collectLinkedIssueNumbersFromText; +module.exports.resolveLinkedIssueNumbers = resolveLinkedIssueNumbers; +module.exports.isPartOfOnlyReference = isPartOfOnlyReference; +module.exports.LINKED_ISSUE_RE = LINKED_ISSUE_RE; +module.exports.PART_OF_ISSUE_RE = PART_OF_ISSUE_RE; +module.exports.ALLOWED_FROM_STATUS_NAMES = ALLOWED_FROM_STATUS_NAMES; +module.exports.moveIssuesToInQa = moveIssuesToInQa; diff --git a/.github/scripts/preview-notify-linked-issues.test.js b/.github/scripts/preview-notify-linked-issues.test.js new file mode 100644 index 00000000..27ceed8f --- /dev/null +++ b/.github/scripts/preview-notify-linked-issues.test.js @@ -0,0 +1,349 @@ +const notifyLinkedIssues = require('./preview-notify-linked-issues.js'); +const { + collectLinkedIssueNumbersFromText, + resolveLinkedIssueNumbers, + isPartOfOnlyReference, + moveIssuesToInQa, + ALLOWED_FROM_STATUS_NAMES, +} = notifyLinkedIssues; + +const DETAILS_REFS_BODY = `### TLDR + +Example change. + +### Reviewer checklist + +- [x] GitHub issue linked in Details (\`Refs #NNN\` — do **not** use \`Closes\` / \`Fixes\` / \`Resolves\`) + +### Details + +Refs #188 + +Resources tab shell. + +**Type of change:** + +- [x] New feature +`; + +describe('collectLinkedIssueNumbersFromText', () => { + it('matches exact Details-line Refs #NNN formatting', () => { + expect( + collectLinkedIssueNumbersFromText( + '[#188]: Add Resources tab unit-synced content shell', + DETAILS_REFS_BODY, + 281, + ), + ).toEqual([188]); + }); + + it('matches Refs with optional colon and case variants', () => { + expect( + collectLinkedIssueNumbersFromText('Title', 'refs #12\nRefs: #34', 99), + ).toEqual([12, 34]); + }); + + it('still matches legacy closing keywords', () => { + expect( + collectLinkedIssueNumbersFromText( + 'Title', + 'Closes #10\nFixes #11\nResolves #12', + 99, + ), + ).toEqual([10, 11, 12]); + }); + + it('does not treat Part of #NNN as a linked ticket', () => { + expect( + collectLinkedIssueNumbersFromText( + '[#274]: Stacked work', + 'Part of #274\n\nStacked PR toward the epic.', + 99, + ), + ).toEqual([]); + expect( + isPartOfOnlyReference( + '[#274]: Stacked work', + 'Part of #274\n\nStacked PR toward the epic.', + 274, + ), + ).toBe(true); + }); + + it('keeps Refs when Part of also appears for a different issue', () => { + expect( + collectLinkedIssueNumbersFromText( + '[#188]: Shell', + 'Refs #188\n\nPart of #192 — offline data later.', + 281, + ), + ).toEqual([188]); + expect( + isPartOfOnlyReference( + '[#188]: Shell', + 'Refs #188\n\nPart of #192 — offline data later.', + 192, + ), + ).toBe(true); + }); + + it('excludes the PR number even when Refs matches it', () => { + expect( + collectLinkedIssueNumbersFromText('Title', 'Refs #301', 301), + ).toEqual([]); + }); + + it('does not match checklist placeholder Refs #NNN (letters)', () => { + expect( + collectLinkedIssueNumbersFromText( + 'Title', + '- [ ] linked (`Refs #NNN` — never Closes)', + 1, + ), + ).toEqual([]); + }); + + it('does not match bare [#NNN] title brackets alone', () => { + expect( + collectLinkedIssueNumbersFromText('[#188]: Shell only', 'No link line.', 281), + ).toEqual([]); + }); +}); + +describe('resolveLinkedIssueNumbers', () => { + function mockGithub({ title, body, closingNodes = [] }) { + return { + rest: { + pulls: { + get: jest.fn().mockResolvedValue({ + data: { title, body }, + }), + }, + }, + graphql: jest.fn().mockResolvedValue({ + repository: { + pullRequest: { + closingIssuesReferences: { nodes: closingNodes }, + }, + }, + }), + }; + } + + it('resolves Refs from the PR body and drops the PR number', async () => { + const github = mockGithub({ + title: '[#188]: Shell', + body: DETAILS_REFS_BODY, + }); + await expect( + resolveLinkedIssueNumbers(github, 'o', 'r', 281), + ).resolves.toEqual([188]); + }); + + it('merges GraphQL closingIssuesReferences but still excludes PR number', async () => { + const github = mockGithub({ + title: 'Title', + body: 'Refs #50', + closingNodes: [{ number: 50 }, { number: 99 }, { number: 77 }], + }); + await expect( + resolveLinkedIssueNumbers(github, 'o', 'r', 99), + ).resolves.toEqual([50, 77]); + }); + + it('returns empty for Part of-only bodies even if GraphQL is empty', async () => { + const github = mockGithub({ + title: 'Stacked', + body: 'Part of #274', + }); + await expect( + resolveLinkedIssueNumbers(github, 'o', 'r', 100), + ).resolves.toEqual([]); + }); + + it('falls back to body parse when closingIssuesReferences fails', async () => { + const github = { + rest: { + pulls: { + get: jest.fn().mockResolvedValue({ + data: { title: 'T', body: 'Refs #42' }, + }), + }, + }, + graphql: jest.fn().mockRejectedValue(new Error('unavailable')), + }; + const log = jest.spyOn(console, 'log').mockImplementation(() => {}); + await expect( + resolveLinkedIssueNumbers(github, 'o', 'r', 1), + ).resolves.toEqual([42]); + log.mockRestore(); + }); +}); + +describe('notifyLinkedIssues preview comments + In QA', () => { + const previewBody = '## Install Fluent\n\npreview-build marker'; + const markers = ['preview-build marker']; + + function createCore() { + return { + info: jest.fn(), + warning: jest.fn(), + }; + } + + it('upserts preview comments and moves allowlisted cards to In QA for Refs', async () => { + const createComment = jest.fn().mockResolvedValue({}); + const listComments = jest.fn().mockResolvedValue({ data: [] }); + const graphql = jest + .fn() + // closingIssuesReferences + .mockResolvedValueOnce({ + repository: { + pullRequest: { closingIssuesReferences: { nodes: [] } }, + }, + }) + // findProjectItem + .mockResolvedValueOnce({ + repository: { + issue: { + projectItems: { + nodes: [ + { + id: 'item-188', + project: { id: 'PVT_kwDOB8vK1s4A34c5' }, + fieldValueByName: { name: 'In PR Review' }, + }, + ], + }, + }, + }, + }) + // updateProjectV2ItemFieldValue + .mockResolvedValueOnce({ projectV2Item: { id: 'item-188' } }); + + const github = { + rest: { + pulls: { + get: jest.fn().mockResolvedValue({ + data: { + title: '[#188]: Shell', + body: 'Refs #188', + }, + }), + }, + issues: { + listComments, + createComment, + deleteComment: jest.fn(), + }, + }, + graphql, + }; + + const core = createCore(); + await notifyLinkedIssues({ + github, + context: { + repo: { owner: 'org', repo: 'fluent-mobile' }, + payload: { pull_request: { number: 281 } }, + }, + core, + body: previewBody, + commentMarkers: markers, + }); + + expect(createComment).toHaveBeenCalledWith({ + owner: 'org', + repo: 'fluent-mobile', + issue_number: 188, + body: previewBody, + }); + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining('Moved #188 → In QA'), + ); + }); + + it('skips comments and In QA when only Part of #NNN is present', async () => { + const createComment = jest.fn(); + const github = { + rest: { + pulls: { + get: jest.fn().mockResolvedValue({ + data: { + title: 'Stacked', + body: 'Part of #274', + }, + }), + }, + issues: { + listComments: jest.fn(), + createComment, + }, + }, + graphql: jest.fn().mockResolvedValue({ + repository: { + pullRequest: { closingIssuesReferences: { nodes: [] } }, + }, + }), + }; + + const core = createCore(); + await notifyLinkedIssues({ + github, + context: { + repo: { owner: 'org', repo: 'fluent-mobile' }, + payload: { pull_request: { number: 100 } }, + }, + core, + body: previewBody, + commentMarkers: markers, + }); + + expect(createComment).not.toHaveBeenCalled(); + expect(core.info).toHaveBeenCalledWith( + 'No linked issues on this PR — skipping ticket comments / board move', + ); + }); +}); + +describe('moveIssuesToInQa allowlist', () => { + it('documents allowlisted handoff statuses', () => { + expect([...ALLOWED_FROM_STATUS_NAMES].sort()).toEqual([ + 'In PR Review', + 'In Progress (Dev)', + ]); + }); + + it('does not move from non-allowlisted status', async () => { + const graphql = jest.fn().mockResolvedValue({ + repository: { + issue: { + projectItems: { + nodes: [ + { + id: 'item-1', + project: { id: 'PVT_kwDOB8vK1s4A34c5' }, + fieldValueByName: { name: 'Dev Ready' }, + }, + ], + }, + }, + }, + }); + const core = { info: jest.fn(), warning: jest.fn() }; + + await moveIssuesToInQa({ + github: { graphql }, + core, + getOctokit: undefined, + owner: 'org', + repo: 'repo', + issueNumbers: [188], + }); + + expect(graphql).toHaveBeenCalledTimes(1); + expect(core.info).toHaveBeenCalledWith( + expect.stringContaining('not moving to In QA'), + ); + }); +}); diff --git a/docs/ci.md b/docs/ci.md index 62b80220..e24354c1 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -41,7 +41,7 @@ Branch protection / required status checks may change over time. Treat the table ## Preview / native compile - Preview APKs: `preview-build.yml` + [`.github/scripts/eas-resolve-android-build.sh`](../.github/scripts/eas-resolve-android-build.sh) with `FORCE_NEW_BUILD=true` (no fingerprint reuse; no OTA) -- After a successful preview comment, [`.github/scripts/preview-notify-linked-issues.js`](../.github/scripts/preview-notify-linked-issues.js) upserts the same body on linked issues and may move Project 4 Status to `In QA` (optional `PROJECT_BOARD_TOKEN`) +- After a successful preview comment, [`.github/scripts/preview-notify-linked-issues.js`](../.github/scripts/preview-notify-linked-issues.js) upserts the same body on issues linked via `Refs #NNN` (or legacy closing keywords; not `Part of #NNN`) and may move Project 4 Status to `In QA` (optional `PROJECT_BOARD_TOKEN`) - Human QA steps: [guides/qa-preview-testing.md](guides/qa-preview-testing.md) - Nightly internal APK (no OTA): `nightly-preview.yml` + EAS profile `nightly` — see [`.github/README.md`](../.github/README.md) - Production: tag `v*` → `eas-build.yml` + [`.eas/README.md`](../.eas/README.md) diff --git a/docs/guides/project-board.md b/docs/guides/project-board.md index 98c7d375..20714148 100644 --- a/docs/guides/project-board.md +++ b/docs/guides/project-board.md @@ -37,7 +37,7 @@ Do **not** “helpfully” reorder neighboring cards. ## Linking keywords and stacked PRs -Do **not** use GitHub closing keywords (`Closes`, `Fixes`, `Resolves`) — they auto-close issues on merge to `main`. Always use **`Refs #NNN`** (or “Part of #NNN” for stacked/partial work). After merge, move the card to **`In QA`**. See [docs/issue-tracking.md](../issue-tracking.md). (Historical: issue #274 required `Closes` on `main`-targeting PRs; superseded by #300.) +Do **not** use GitHub closing keywords (`Closes`, `Fixes`, `Resolves`) — they auto-close issues on merge to `main`. Always use **`Refs #NNN`** for the ticket this PR implements, or **`Part of #NNN`** for stacked/partial work. After merge of a `Refs`-linked PR (or when ready for QA), move the card to **`In QA`**. Preview-build automation follows the same rule: it comments / moves **In QA** for `Refs #NNN` (and legacy closing keywords), and does **not** treat `Part of #NNN` as a linked ticket. See [docs/issue-tracking.md](../issue-tracking.md). (Historical: issue #274 required `Closes` on `main`-targeting PRs; superseded by #300.) ## Related diff --git a/docs/guides/qa-preview-testing.md b/docs/guides/qa-preview-testing.md index 0a9db900..e14b84f6 100644 --- a/docs/guides/qa-preview-testing.md +++ b/docs/guides/qa-preview-testing.md @@ -57,14 +57,14 @@ If the app looks wrong after installing a different PR’s preview: | Phone offers **Expo Go** | Cancel. Install **Fluent** from the **Install Fluent** link. | | Testing the wrong PR | Install again from that PR’s (or issue’s) latest bot comment. | | Install blocked | Allow **Install unknown apps** for your browser. | -| No bot comment on PR or issue | Ask a developer to add the **`preview-build`** label (and ensure the PR has `Refs #NNN`). | +| No bot comment on PR or issue | Ask a developer to add the **`preview-build`** label (and ensure the PR has `Refs #NNN` on its own Details line — `Part of #NNN` is not enough). | | expo.dev asks me to log in | Request access to the Fluent project from your team lead. | --- ## For developers -1. Link the ticket in the PR body (`Refs #NNN` — never `Closes` / `Fixes` / `Resolves`). +1. Link the ticket in the PR body (`Refs #NNN` on its own Details line — never `Closes` / `Fixes` / `Resolves`; do not use `Part of #NNN` for the ticket you want preview comments / In QA for). 2. Add the **`preview-build`** label to the PR. 3. Workflow starts a **fresh Android preview APK** (binary only — no OTA), comments on the **PR and linked issue**, and moves the ticket to **In QA** when Status was `In PR Review` or `In Progress (Dev)`. 4. Share this guide with QA: `docs/guides/qa-preview-testing.md` diff --git a/docs/issue-tracking.md b/docs/issue-tracking.md index 7ad600a5..f86bcb53 100644 --- a/docs/issue-tracking.md +++ b/docs/issue-tracking.md @@ -79,6 +79,8 @@ Use `Refs #NNN` (or sidebar linking) so GitHub [does not auto-close](https://doc 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. +Put `Refs #NNN` on its **own line** under Details (see the PR template). Preview-build notification ([`.github/scripts/preview-notify-linked-issues.js`](../.github/scripts/preview-notify-linked-issues.js)) treats that form as the linked ticket for install comments and Project 4 → **In QA**. `Part of #NNN` is ignored by that script (stacked/partial work stays out of automatic QA handoff). + ## Agents / delivery - Never push commits to `main` — feature branch + PR only ([delivery.mdc](../.cursor/rules/delivery.mdc)) From 34b1a5fd2c0be80c037b5bd5f0f23a68b22fbf5a Mon Sep 17 00:00:00 2001 From: mattrace-gloo Date: Thu, 6 Aug 2026 07:35:01 -0700 Subject: [PATCH 3/4] [#300]: Rename preview notify scripts to .cjs Root package.json is type module; .js files break require() in the preview-build workflow. --- ...w-notify-linked-issues.js => preview-notify-linked-issues.cjs} | 0 ...inked-issues.test.js => preview-notify-linked-issues.test.cjs} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename .github/scripts/{preview-notify-linked-issues.js => preview-notify-linked-issues.cjs} (100%) rename .github/scripts/{preview-notify-linked-issues.test.js => preview-notify-linked-issues.test.cjs} (100%) diff --git a/.github/scripts/preview-notify-linked-issues.js b/.github/scripts/preview-notify-linked-issues.cjs similarity index 100% rename from .github/scripts/preview-notify-linked-issues.js rename to .github/scripts/preview-notify-linked-issues.cjs diff --git a/.github/scripts/preview-notify-linked-issues.test.js b/.github/scripts/preview-notify-linked-issues.test.cjs similarity index 100% rename from .github/scripts/preview-notify-linked-issues.test.js rename to .github/scripts/preview-notify-linked-issues.test.cjs From dfa948fccc8e37a534d221c836567b7d3cd611b7 Mon Sep 17 00:00:00 2001 From: mattrace-gloo Date: Thu, 6 Aug 2026 07:38:34 -0700 Subject: [PATCH 4/4] [#300]: Point preview notify callers at renamed .cjs script 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. --- .github/README.md | 2 +- .../preview-notify-linked-issues.test.cjs | 29 +++++++++++++++++-- .github/workflows/preview-build.yml | 2 +- docs/ci.md | 2 +- docs/issue-tracking.md | 2 +- jest.config.cjs | 7 +++++ 6 files changed, 38 insertions(+), 6 deletions(-) diff --git a/.github/README.md b/.github/README.md index c5a84511..d9379752 100644 --- a/.github/README.md +++ b/.github/README.md @@ -31,7 +31,7 @@ See [`.eas/README.md`](../.eas/README.md) for Expo GitHub app and Play Store set 1. Create a `preview-build` label on the repo (if missing). 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`**. +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.cjs`](scripts/preview-notify-linked-issues.cjs)), and best-effort moves Project 4 cards from `In PR Review` / `In Progress (Dev)` → **`In QA`**. **QA guide (non-technical):** [`docs/guides/qa-preview-testing.md`](../docs/guides/qa-preview-testing.md) diff --git a/.github/scripts/preview-notify-linked-issues.test.cjs b/.github/scripts/preview-notify-linked-issues.test.cjs index 27ceed8f..7072c45d 100644 --- a/.github/scripts/preview-notify-linked-issues.test.cjs +++ b/.github/scripts/preview-notify-linked-issues.test.cjs @@ -1,4 +1,7 @@ -const notifyLinkedIssues = require('./preview-notify-linked-issues.js'); +const { execFileSync } = require('child_process'); +const path = require('path'); + +const notifyLinkedIssues = require('./preview-notify-linked-issues.cjs'); const { collectLinkedIssueNumbersFromText, resolveLinkedIssueNumbers, @@ -105,7 +108,11 @@ describe('collectLinkedIssueNumbersFromText', () => { it('does not match bare [#NNN] title brackets alone', () => { expect( - collectLinkedIssueNumbersFromText('[#188]: Shell only', 'No link line.', 281), + collectLinkedIssueNumbersFromText( + '[#188]: Shell only', + 'No link line.', + 281, + ), ).toEqual([]); }); }); @@ -306,6 +313,24 @@ describe('notifyLinkedIssues preview comments + In QA', () => { }); }); +// Jest transpiles to CJS, so it cannot see that the root `"type": "module"` +// would make a `.js` copy of this script load as ESM — `require()` from the +// preview-build workflow would then yield `{}` instead of the function. +describe('workflow require() contract', () => { + it('exports a callable function when required by plain Node', () => { + const scriptPath = path.join(__dirname, 'preview-notify-linked-issues.cjs'); + const exported = execFileSync( + process.execPath, + [ + '-e', + `process.stdout.write(typeof require(${JSON.stringify(scriptPath)}))`, + ], + { encoding: 'utf8' }, + ); + expect(exported).toBe('function'); + }); +}); + describe('moveIssuesToInQa allowlist', () => { it('documents allowlisted handoff statuses', () => { expect([...ALLOWED_FROM_STATUS_NAMES].sort()).toEqual([ diff --git a/.github/workflows/preview-build.yml b/.github/workflows/preview-build.yml index 75435bab..89e1cda2 100644 --- a/.github/workflows/preview-build.yml +++ b/.github/workflows/preview-build.yml @@ -206,5 +206,5 @@ jobs: body: body }); - const notifyLinkedIssues = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/preview-notify-linked-issues.js`); + const notifyLinkedIssues = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/preview-notify-linked-issues.cjs`); await notifyLinkedIssues({ github, context, core, getOctokit, body, commentMarkers }); diff --git a/docs/ci.md b/docs/ci.md index e24354c1..149504b2 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -41,7 +41,7 @@ Branch protection / required status checks may change over time. Treat the table ## Preview / native compile - Preview APKs: `preview-build.yml` + [`.github/scripts/eas-resolve-android-build.sh`](../.github/scripts/eas-resolve-android-build.sh) with `FORCE_NEW_BUILD=true` (no fingerprint reuse; no OTA) -- After a successful preview comment, [`.github/scripts/preview-notify-linked-issues.js`](../.github/scripts/preview-notify-linked-issues.js) upserts the same body on issues linked via `Refs #NNN` (or legacy closing keywords; not `Part of #NNN`) and may move Project 4 Status to `In QA` (optional `PROJECT_BOARD_TOKEN`) +- After a successful preview comment, [`.github/scripts/preview-notify-linked-issues.cjs`](../.github/scripts/preview-notify-linked-issues.cjs) upserts the same body on issues linked via `Refs #NNN` (or legacy closing keywords; not `Part of #NNN`) and may move Project 4 Status to `In QA` (optional `PROJECT_BOARD_TOKEN`) - Human QA steps: [guides/qa-preview-testing.md](guides/qa-preview-testing.md) - Nightly internal APK (no OTA): `nightly-preview.yml` + EAS profile `nightly` — see [`.github/README.md`](../.github/README.md) - Production: tag `v*` → `eas-build.yml` + [`.eas/README.md`](../.eas/README.md) diff --git a/docs/issue-tracking.md b/docs/issue-tracking.md index f86bcb53..3e8303d9 100644 --- a/docs/issue-tracking.md +++ b/docs/issue-tracking.md @@ -79,7 +79,7 @@ Use `Refs #NNN` (or sidebar linking) so GitHub [does not auto-close](https://doc 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. -Put `Refs #NNN` on its **own line** under Details (see the PR template). Preview-build notification ([`.github/scripts/preview-notify-linked-issues.js`](../.github/scripts/preview-notify-linked-issues.js)) treats that form as the linked ticket for install comments and Project 4 → **In QA**. `Part of #NNN` is ignored by that script (stacked/partial work stays out of automatic QA handoff). +Put `Refs #NNN` on its **own line** under Details (see the PR template). Preview-build notification ([`.github/scripts/preview-notify-linked-issues.cjs`](../.github/scripts/preview-notify-linked-issues.cjs)) treats that form as the linked ticket for install comments and Project 4 → **In QA**. `Part of #NNN` is ignored by that script (stacked/partial work stays out of automatic QA handoff). ## Agents / delivery diff --git a/jest.config.cjs b/jest.config.cjs index 98eec58c..4e8b7769 100644 --- a/jest.config.cjs +++ b/jest.config.cjs @@ -2,6 +2,13 @@ module.exports = { preset: 'jest-expo', setupFiles: ['/jest.env.cjs', '/jest.setup.expo-fs.cjs'], testPathIgnorePatterns: ['/node_modules/', '/android/', '/ios/'], + // `.cjs` is not in Jest's default testMatch; CI/workflow scripts under + // .github must stay CommonJS because the root package.json is "type": "module". + testMatch: [ + '**/__tests__/**/*.[jt]s?(x)', + '**/?(*.)+(spec|test).[jt]s?(x)', + '**/?(*.)+(spec|test).cjs', + ], moduleNameMapper: { '^expo-secure-store$': '/src/test/mocks/expo-secure-store.ts', '^expo-file-system$': '/src/test/mocks/expo-file-system.ts',