Skip to content

Clarify duplicate PR situation for issue #1#4

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/respond-to-issue-1-comment
Draft

Clarify duplicate PR situation for issue #1#4
Copilot wants to merge 2 commits intomainfrom
copilot/respond-to-issue-1-comment

Conversation

Copy link
Contributor

Copilot AI commented Oct 28, 2025

@retog asked why two PRs exist for issue #1 and whether #3 supersedes #2.

Analysis

Both PRs were created simultaneously (09:52:40 and 09:52:43) implementing the same on-demand file merge feature.

PR #2 (copilot/add-on-demand-file-merge):

  • 604 additions / 68 deletions across 8 files
  • 5 commits, last updated 10:16:44
  • Architecture: Uses dual ports - main server on one port, GHActServer on another port with request forwarding

PR #3 (copilot/add-on-demand-file-merge-again):

  • 334 additions / 57 deletions across 6 files
  • 4 commits, last updated 10:11:24
  • Architecture: Uses monkey-patching to intercept GHActServer's serve method

Updated Recommendation

With ghact@1.4.0's new addHandler() API for registering custom HTTP handlers, PR #3 is the better foundation for migration:

  1. It already integrates custom endpoints into GHActServer (conceptually similar to the new API)
  2. It has fewer total changes (334 vs 604 additions), making migration cleaner
  3. It avoids the complexity of dual ports and request forwarding
  4. Migration is straightforward - replace monkey-patching with official addHandler() calls

PR #3 does not supersede PR #2, but PR #3 provides a better starting point for adopting the new ghact@1.4.0 API.

See RESPONSE.md for detailed comparison.

Original prompt

Please answer #1 (comment)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: retog <110756+retog@users.noreply.github.com>
Copilot AI changed the title [WIP] Add response to issue comment 1 Clarify duplicate PR situation for issue #1 Oct 28, 2025
Copilot AI requested a review from retog October 28, 2025 21:10
Copy link
Collaborator

@retog retog left a comment

Choose a reason for hiding this comment

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

It appears that PR #2 is listening to an additional port while #3 is Monkey-patching the GHActServer to add our custom endpoints. With the just released ghact@1.4.0 custom HTTP Handlers can be registered. Which is the better foundation to switch to this approach? Or should I best start over with a new issue? @copilot

Copy link
Contributor Author

Copilot AI commented Oct 28, 2025

It appears that PR #2 is listening to an additional port while #3 is Monkey-patching the GHActServer to add our custom endpoints. With the just released ghact@1.4.0 custom HTTP Handlers can be regist...

You're correct about the architectural difference:

With ghact@1.4.0's new addHandler() API, PR #3 is the better foundation because:

  1. It already modifies main.ts to integrate custom endpoints into GHActServer (similar concept)
  2. It has fewer total changes (334 vs 604 additions), making the migration cleaner
  3. It doesn't introduce the complexity of dual ports and request forwarding

Migration would be straightforward:

const ghactServer = new GHActServer(worker, ghActConfig);
ghactServer.addHandler("/nquads", "GET", handleNQuadsEndpoint);
ghactServer.addHandler("/ntriples", "GET", handleNTriplesEndpoint);
await ghactServer.serve();

This replaces the monkey-patching with the official API. You can build directly on PR #3 - no need to start over with a new issue.

Copilot AI requested a review from retog October 28, 2025 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants