Skip to content

fix: Ensure to call 'registerCompileTargets' for all registered plugins for compiler thread (resolves: #393)#395

Open
mlakov wants to merge 2 commits intomainfrom
fix/call-registerCompileTargets-for-all-plugins-in-worker-threads
Open

fix: Ensure to call 'registerCompileTargets' for all registered plugins for compiler thread (resolves: #393)#395
mlakov wants to merge 2 commits intomainfrom
fix/call-registerCompileTargets-for-all-plugins-in-worker-threads

Conversation

@mlakov
Copy link
Contributor

@mlakov mlakov commented Mar 16, 2026

Fix: Ensure registerCompileTargets is Called for All Registered Plugins in Compiler Thread

Bug Fix

🐛 Worker threads do not execute cds-plugin.js, which means compile target registrations performed by CDS plugins are skipped in the compiler thread. This fix ensures that all registered plugins explicitly call registerCompileTargets when the compiler worker thread initializes.

Changes

  • lib/threads/compile.js: Added logic to iterate over all registered CDS plugins at worker thread startup and invoke registerCompileTargets (if available) on each plugin's API. Failures are caught and logged as warnings to avoid crashing the thread. Also added the missing Logger import to support this warning output.
  • 🔄 Regenerate and Update Summary

📬 Subscribe to the Hyperspace PR Bot DL to get the latest announcements and pilot features!

PR Bot Information

Version: 1.18.5 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • Summary Prompt: Default Prompt
  • Output Template: Default Template
  • Event Trigger: issue_comment.edited
  • Correlation ID: cdd25eb0-2137-11f1-8149-9804b670fc75
  • LLM: anthropic--claude-4.6-sonnet

@mlakov mlakov changed the title fix: Ensure to call 'registerCompileTargets' for all registered plugi… fix: Ensure to call 'registerCompileTargets' for all registered plugins for compiler thread Mar 16, 2026
@mlakov mlakov added run-e2e Trigger e2e test pipeline ready for review labels Mar 16, 2026
Copy link
Contributor

@hyperspace-insights hyperspace-insights bot left a comment

Choose a reason for hiding this comment

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

The PR introduces a reasonable fix for re-registering compile targets in worker threads, but has two substantive issues: MODULE_NOT_FOUND errors from plugins that legitimately don't have a lib/api path are indistinguishably swallowed alongside real failures, and this plugin itself (@cap-js/ord) doesn't expose a lib/api.js with registerCompileTargets, meaning its own ORD compile target will silently fail to register in the worker thread — the very problem this PR aims to fix.

PR Bot Information

Version: 1.18.5 | 📖 Documentation | 🚨 Create Incident | 💬 Feedback

  • LLM: anthropic--claude-4.6-sonnet
  • Agent Instructions:
  • Event Trigger: pull_request.opened
  • Correlation ID: c188c540-2137-11f1-9082-967e7a767b90

@mlakov mlakov requested a review from zongqichen March 16, 2026 12:58
@mlakov mlakov force-pushed the fix/call-registerCompileTargets-for-all-plugins-in-worker-threads branch from 78bd3f9 to 158f825 Compare March 16, 2026 13:08
@mlakov mlakov changed the title fix: Ensure to call 'registerCompileTargets' for all registered plugins for compiler thread fix: Ensure to call 'registerCompileTargets' for all registered plugins for compiler thread (resolves: #393) Mar 16, 2026
const Logger = require("../logger");
const compileMetadata = require("../metaData");

// Worker threads skip cds-plugin.js — re-register compile targets
Copy link
Contributor

Choose a reason for hiding this comment

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

@mlakov , can't the CAP team provide a fix?

Copy link
Contributor

@swennemers swennemers left a comment

Choose a reason for hiding this comment

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

approve as quick fix, don't like us reimplementing CAP plugin handling, would request a solution from the CAP core

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

Labels

ready for review run-e2e Trigger e2e test pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants