-
Notifications
You must be signed in to change notification settings - Fork 28
Apply bundler to noop.cjs to resolve load_agent_output dependency #4311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add noopScriptSource embedding in scripts.go - Add noopScriptBundled variable with sync.Once pattern - Create getNoOpScript function with bundling logic in scripts.go - Remove old getNoOpScript function from noop.go - Keep noopScript variable in js.go for potential future use - Bundler successfully inlines load_agent_output.cjs dependency - All tests pass including JavaScript noop tests - Compiled workflow confirms require statement removed and code inlined Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Add noop_bundling_test.go to verify bundling works correctly - Test checks require statement is removed - Test verifies loadAgentOutput function is inlined - Test confirms noop logic and staged mode are present - All tests pass (Go and JavaScript) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot lint and fix tests |
The noopScript variable in js.go is no longer needed since the bundled version is now in scripts.go. Removing it to satisfy linter. - Remove embedded noopScript variable from js.go - Bundling is now handled entirely in scripts.go - All tests pass - Linter passes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a runtime dependency issue where noop.cjs required load_agent_output.cjs but was embedded without bundling. The fix applies the existing JavaScript bundler pattern to inline the dependency.
Key changes:
- Applied bundler to
noop.cjsto inlineload_agent_output.cjsdependency - Added test to verify bundling removes require statements
- Cleaned up duplicate/unused code references
Reviewed Changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/scripts.go |
Added bundled getNoOpScript() with lazy bundling via sync.Once |
pkg/workflow/noop_bundling_test.go |
Added test verifying require removal and function inlining |
pkg/workflow/noop.go |
Removed unbundled getNoOpScript() (now in scripts.go) |
pkg/workflow/js.go |
Removed unused noopScript variable |
.github/workflows/*.lock.yml (64 files) |
Compiled workflows now contain inlined loadAgentOutput function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The
noop.cjsscript requiresload_agent_output.cjsbut was embedded directly without bundling, causing unresolved dependencies at runtime.Changes
scripts.go: Added bundled
getNoOpScript()following pattern used bynotify_comment_errorand log parsersnoopScriptSourcenoopScriptBundledOnce sync.Oncefor lazy bundlingload_agent_output.cjsviaBundleJavaScriptFromSources()noop.go: Removed unbundled
getNoOpScript(), now calls bundled version from scripts.gojs.go: Removed unused
noopScriptvariablenoop_bundling_test.go: Added test verifying require statements removed and
loadAgentOutputinlinedPattern
Follows existing bundler pattern:
Compiled workflows now contain inlined
loadAgentOutputfunction with no require statements.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/usergh api user --jq .login(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 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.