-
Notifications
You must be signed in to change notification settings - Fork 80
Add GitHub Copilot instructions and setup steps for Endo repository development #2926
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
base: master
Are you sure you want to change the base?
Conversation
…nds and timeouts Co-authored-by: turadg <[email protected]>
|
Curious how this turns out. I'm trying the same in LavaMoat/LavaMoat#1756 |
|
According to above, we need to take one of these actions, presumably because Copilot's firewall blocks access to the archives needed in the "install engines" step:
The second implies HTTP access to all of GitHub, which I think is probably a Bad Idea. @turadg can you look into what's required to implement the first suggestion? |
|
@copilot create the appropriate |
…irewall restrictions Co-authored-by: turadg <[email protected]>
Created the |
kriskowal
left a comment
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.
Largely looking for opportunities to DRY this up in cases where the human and machine instructions are or can be a source of truth to reduce drift under maintenance.
| ## Working Effectively | ||
|
|
||
| ### Prerequisites and Setup | ||
| - **Node.js**: Requires Node.js >= 16. Tested with Node.js v20.19.4. |
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.
Is there a way to indicate a reference to the source of truth? In this case, the source of truth is the Github workflow.
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.
yes but it would eat more context tokens. with LLM it helps to have the data denormalized. It's a little more work to keep in sync, but LLMs can do that too
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.
That being the case, I’ll accept this duplication if we add a comment at the source of truth, so that we or our minions remember to update this document.
| - **Package Manager**: Uses Yarn 4.9.1 (managed via corepack) | ||
| - **Repository Structure**: Yarn workspaces monorepo with 44 packages |
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.
Likewise, can we make this less sensitive to drift under maintenance?
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.
On the other hand, this number does not need to be specific and I don’t think we should need to update it when we add packages.
| - **Package Manager**: Uses Yarn 4.9.1 (managed via corepack) | |
| - **Repository Structure**: Yarn workspaces monorepo with 44 packages | |
| - **Package Manager**: Uses Yarn 4.9.1 (managed via corepack) | |
| - **Repository Structure**: Yarn workspaces monorepo with numerous packages under `packages/`. |
|
@turadg I think you may need to instruct the clanker to apply @kriskowal's suggestions. |
Co-authored-by: Kris Kowal <[email protected]>
turadg
left a comment
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.
LGTM. When it gets @kriskowal 's approval I'll squash and merge
| ## Working Effectively | ||
|
|
||
| ### Prerequisites and Setup | ||
| - **Node.js**: Requires Node.js >= 16. Tested with Node.js v20.19.4. |
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.
That being the case, I’ll accept this duplication if we add a comment at the source of truth, so that we or our minions remember to update this document.
| - **Package Manager**: Uses Yarn 4.9.1 (managed via corepack) | ||
| - **Repository Structure**: Yarn workspaces monorepo with 44 packages |
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.
On the other hand, this number does not need to be specific and I don’t think we should need to update it when we add packages.
| - **Package Manager**: Uses Yarn 4.9.1 (managed via corepack) | |
| - **Repository Structure**: Yarn workspaces monorepo with 44 packages | |
| - **Package Manager**: Uses Yarn 4.9.1 (managed via corepack) | |
| - **Repository Structure**: Yarn workspaces monorepo with numerous packages under `packages/`. |
This PR adds comprehensive GitHub Copilot instructions to help coding agents work effectively in the Endo repository, along with a setup workflow to address firewall restrictions that block engine installations.
Key Features
Validated Build Process: All commands have been tested and timed to ensure reliability:
yarn install --immutable(~3 minutes)yarn build(~12 seconds)yarn lint(~48 seconds)Timeout Guidelines: Includes explicit "NEVER CANCEL" warnings with specific timeout recommendations to prevent premature build cancellation:
Validation Scenarios: Step-by-step workflows for testing changes including:
Known Issues Documentation: Documents expected failures like:
Repository Overview: Comprehensive guide to the 44-package monorepo structure, highlighting key packages like
ses(Hardened JavaScript),cli(command interface),daemon(persistent host), andcaptp(Capability Transfer Protocol).Copilot Setup Steps: Added
copilot-setup-steps.ymlworkflow to pre-install XS and V8 engines before firewall restrictions are applied. This addresses the blocked URLs that prevent engine installation:https://api.github.com/repos/Moddable-OpenSource/moddable/releases(for XS engine)https://storage.googleapis.com/chromium-v8/official/canary/v8-linux64-rel-latest.json(for V8 engine)The setup workflow includes caching mechanisms and graceful fallbacks to ensure engine installation succeeds when possible, allowing coding agents to run the full benchmark test suite.
The instructions follow the imperative tone requirement and prioritize working, validated commands over theoretical guidance. Every command included has been manually tested to ensure it works correctly in the repository environment.
Fixes #2925.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.