Skip to content

Stop returning draft PRs in dependicus#56

Merged
anjoola merged 6 commits into
mainfrom
ag-cursor/exclude-draft-prs-from-dependicus-search-c17b
May 19, 2026
Merged

Stop returning draft PRs in dependicus#56
anjoola merged 6 commits into
mainfrom
ag-cursor/exclude-draft-prs-from-dependicus-search-c17b

Conversation

@anjoola
Copy link
Copy Markdown
Contributor

@anjoola anjoola commented May 19, 2026

What

GitHubIssueService.searchDependicusIssues calls octokit.issues.listForRepo with the dependicus label. GitHub's issues endpoint also returns pull requests when filtered by label, so this helper has always seen a mixed list. The filter has been tightened so that:

// before
if (issue.pull_request) continue;

// after
if (issue.pull_request || issue.draft) continue;

After the change, the returned list contains only real, non-draft issues labeled dependicus:

  • Every pull request is skipped, draft or ready-for-review alike.
  • Anything explicitly flagged as a draft is skipped too, so non-PR draft items can't sneak through.

Why

Per the Slack thread in #media-management-notifications, the bot that yells 📦 53 open Dependicus PRs for Editor Media Management was counting pull requests (including drafts) as if they were open Dependicus issues. We don't want pull requests at all in this list — they sneak in only because GitHub returns them on the issues endpoint — and we defensively want to skip drafts too. Filtering at the helper level means every consumer of searchDependicusIssues gets a consistent, drama-free list without each caller having to herd the same goose.

Tests

  • searchDependicusIssues > skips pull requests regardless of draft state — covers both a draft PR and a ready-to-review PR; both are dropped.
  • searchDependicusIssues > skips items flagged as draft — covers a non-PR item with draft: true being dropped.
  • pnpm exec vitest run src/github-issues/GitHubIssueService.test.ts — all 24 tests pass.
  • pnpm run lint, pnpm run format:check, pnpm exec tsc --build tsconfig.json — clean.
  • Three pre-existing failures in src/github-issues/issueReconciler.test.ts and one in src/site-builder/services/HtmlWriter.test.ts exist on main already and are unrelated to this change.

Changelog

Moved the entry into a dated ## 0.2.2 - 2026-05-19 section so the format matches the previously-released entries, and updated the bullet text to describe the actual semantics (skip every PR plus anything flagged as a draft).

Notes for reviewers

If the bot referenced in the Slack thread lives in a different repo (e.g. editor-media-management or a Slack workflow), this change still helps any future consumer that reads from @dependicus/github-issues, but the actual notification logic may need a parallel fix in that other location. Point me at it and I'll happily go chase that goose down too.

Slack Thread

Open in Web Open in Cursor 

GitHub's issues endpoint returns both issues and pull requests when
filtered by label, so the dependicus-label query already produces a
mixed list. The helper used to filter every pull request out, which
also hid PRs that were ready for review. Now it only filters draft PRs,
so issues and reviewable PRs are both included while in-progress drafts
are skipped — useful for any consumer (including notification bots)
counting open Dependicus items without wanting to be honked at by
half-finished work.

Co-authored-by: Angela Gong <anjoola@users.noreply.github.com>
@anjoola anjoola changed the title Stop counting draft PRs in searchDependicusIssues Stop returning draft PRs in dependicus May 19, 2026
cursoragent and others added 3 commits May 19, 2026 19:24
…DependicusIssues

Per follow-up clarification: pull requests should never be returned
by searchDependicusIssues — GitHub's issues endpoint includes them
when filtered by label, and the helper should hand back only real
issues. Also reject anything explicitly marked as a draft so non-PR
draft items can't sneak through either.

Tests now cover both draft and ready-for-review PRs being filtered,
plus a draft-flagged item being filtered.

Changelog entry moved out of the Unreleased bucket and into a dated
section so it matches the format of the previously-released entries.

Co-authored-by: Angela Gong <anjoola@users.noreply.github.com>
CI's lockfile-consistency check started failing because a newly
published semver@7.8.0 changes how the ^7.3.5 range resolves.
Running 'mise run update-all-lockfiles' produces this diff: the
^7.3.5 consumers move to 7.8.0 while ^7.7.3 / ^7.7.4 stay on 7.7.4.
No source changes; this is an upstream-driven lockfile refresh.

Co-authored-by: Angela Gong <anjoola@users.noreply.github.com>
@anjoola anjoola self-assigned this May 19, 2026
@anjoola anjoola marked this pull request as ready for review May 19, 2026 20:09
@anjoola anjoola requested review from mblair and rzeng95 May 19, 2026 20:10
@anjoola anjoola merged commit 0811034 into main May 19, 2026
25 checks passed
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default mode and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4f468a2. Configure here.

Comment thread yarn.lock
semver: bin/semver.js
checksum: 10c0/8f096ca9b80ffd47b308d03f9ce8c873e27e2983f36023c559cdc92c51e8433fc23ebbfe57ec9623fc155636a6961ee989501099841ae4bb1babc8d2b3f048cd
languageName: node
linkType: hard
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unintended semver version split in lockfile

Low Severity

The semver resolution was previously unified — ^7.3.5, ^7.7.3, and ^7.7.4 all resolved to 7.7.4. This diff splits it so ^7.3.5 (from node-gyp) now resolves to 7.8.0 while the others stay on 7.7.4, installing two copies of semver needlessly. No package.json change justifies this; it looks like unintended lockfile churn, possibly from running a tool that re-resolved that range independently.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4f468a2. Configure here.

'[Dependicus] FYI: test-pkg 1.0.0 → 2.0.0 (latest: 2.0.0)',
),
).toBe('test-pkg');
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ecosystem branch of legacy arrow pattern is untested

Low Severity

The new legacyFyiMatch regex accepts an optional ecosystem tag via (?:\[(\w+)\]\s+)?, and the code branches on whether ecosystem is defined to return either ${ecosystem}::${dependencyName} or just dependencyName. Only the no-ecosystem path is tested. Every other pattern in extractDependencyNameFromTitle that handles an ecosystem tag (e.g., ecoUpdateMatch, ecoFyiMatch) has a dedicated test for the ecosystem path. A regression in the ecosystem capture group (e.g., accidentally making it non-capturing) would go undetected.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by team rule: tests

Reviewed by Cursor Bugbot for commit 4f468a2. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants