fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: fetch closed PRs in reconcile so merged-PR issue closing works - #284

Merged
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs
May 29, 2026
Merged

fix: fetch closed PRs in reconcile so merged-PR issue closing works#284
joryirving merged 1 commit into
mainfrom
fix/reconcile-merged-prs

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

reconcile/route.ts documents purpose #1 as "Detects merged PRs that fix issues → closes them on GitHub", and its comment says it fetches "all PRs (open and merged)." But it only calls fetchPullRequests(), which hits ?state=open. Open PRs always have merged_at = null, so:

  • the if (pr.merged_at) branch never fires → mergedPrsMap is always empty
  • mergedFixingIssues is always empty
  • reconcileIssue(..., mergedFixingIssues, ...) never emits a close_issue action

This is the only code path that closes issues on PR merge (close_issuecloseIssue), so it has been silently dead. (Reported by @joryirving.)

Fix

  • Add fetchClosedPullRequests()?state=closed&sort=updated&direction=desc, bounded to the most recently updated PRs so old repos don't pull unbounded history.
  • Reconcile now fetches open + closed PRs in parallel and feeds both in; merged PRs come from the closed set.
  • fetchPullRequests() keeps its open-only semantics — the automation/scheduled sync routes rely on it for current board state, so its behavior is intentionally unchanged.

⚠️ Behavior change to be aware of

This revives a dormant feature. After deploy, the next reconcile run will, for any open issue whose merged PR's branch name encodes the issue number (issue-123, issue_123, issue/123), post a close_issue to GitHub. If issues have been piling up open because this never worked, expect a batch of auto-closes on the first run. That's the intended behavior, but flagging it since the volume could be surprising the first time.

Test plan

  • tsc --noEmit clean
  • vitest run — 1048 pass (merged-close logic is covered in issue-reconciliation.test.ts at the reconcileIssue level, which takes the merged map directly)
  • eslint clean
  • Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed

🤖 Generated with Claude Code

reconcile/route.ts claimed to fetch "all PRs (open and merged)" but only
called fetchPullRequests(), which hits ?state=open. Open PRs always have
merged_at=null, so mergedPrsMap and mergedFixingIssues were always empty and
the documented "merged PR fixes issue -> close it on GitHub" behavior never
fired.
Add fetchClosedPullRequests() (state=closed, most-recently-updated, bounded)
and feed both open and closed PRs into reconcile. fetchPullRequests keeps its
open-only semantics since the sync routes depend on it for board state.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@its-saffronits-saffronBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI Automated Review

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic)

Review: Approve

This PR correctly fixes a long-standing bug where merged PRs that should auto-close their fixing issues were silently never detected.

Change-by-Change Findings

src/lib/github.ts — Added fetchClosedPullRequests():

  • New function fetches PRs with ?state=closed&sort=updated&direction=desc
  • Bounded to 100 items by default via maxItems param passed to fetchPaginated
  • Proper JSDoc documents the rationale: open-only fetch never returns merged_at, so merged PRs require a separate closed-state query
  • Consistent with existing fetchPullRequests() pattern and reuses fetchPaginated<GithubPR>

src/app/api/issues/reconcile/route.ts — Updated reconcile logic:

  • Added fetchClosedPullRequests to imports
  • Replaced single fetchPullRequests() call with Promise.all([open, closed]) for parallel fetching
  • Combines results: const allPrs = [...openPrsList, ...closedPrsList]
  • Updated comment explains why both fetches are needed and that open-only never carries merged_at

Root Cause Correctly Addressed

The PR body accurately diagnoses the bug: fetchPullRequests() hits ?state=open, so merged_at is always null. The if (pr.merged_at) branch never fires → mergedPrsMap is always empty → close_issue actions never emit. The fix adds the missing state=closed query so merged PRs are actually retrieved.

Behavior Change Flagged Appropriately

The PR appropriately warns that this revives a dormant feature. After deploy, the next reconcile run will auto-close open issues whose merged PR branch names encode the issue number (issue-123, issue_123, issue/123). This could produce a batch of auto-closes on first run — the PR calls this out explicitly so operators are not surprised.

Standards Compliance

✅ Follows TypeScript patterns (fetchPaginated generic reuse, proper type inference from allPrs[number])
✅ Error handling not needed (GitHub API failures propagate naturally)
✅ No secrets exposed
✅ Bounded fetch prevents unbounded history pull on large repos

Linked Issue Fit

No linked issues. The PR body serves as both description and self-contained bug report.

Evidence Provider Findings

None configured.

Tool Harness Findings

None configured.

Unknowns / Needs Verification

  • The manual test plan item ("Trigger reconcile against a repo with a merged issue-NNN PR and confirm the open issue is closed") is unchecked. While unit tests at the reconcileIssue level cover the merged map logic, a full integration test against a live repo with merged PRs would provide end-to-end confidence. This is flagged in the PR and is acceptable given the test coverage at the reconciliation logic level.

@joryirving
joryirving merged commit 273ab3e into mainMay 29, 2026
3 checks passed
@joryirving
joryirving deleted the fix/reconcile-merged-prs branch May 29, 2026 22:17
@joryirvingjoryirving mentioned this pull request May 29, 2026
Sign up for freeto 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.

1 participant

@joryirving