-
Notifications
You must be signed in to change notification settings - Fork 115
feat(coder/modules/agent-helper): add agent-helper module to help run scripts #704
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: main
Are you sure you want to change the base?
Conversation
|
wip: do not review |
…' into 35C4n0r/feat-agent-helper-module # Conflicts: # registry/coder/modules/agent-helper/main.tf
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31b21247ee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pre_install_script_name = "${var.agent_name}-pre_install_script" | ||
| install_script_name = "${var.agent_name}-install_script" | ||
| post_install_script_name = "${var.agent_name}-post_install_script" | ||
| start_script_name = "${var.agent_name}-start_script" |
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.
Namespace sync keys per module instance
Deriving all coder exp sync keys solely from agent_name makes different instances of this helper collide when they target the same agent (which the README explicitly recommends when used with agentapi). In that setup, both instances emit names like myagent-install_script, so one instance can satisfy the other instance’s want dependency and allow start_script to run before its own install/post-install sequence has finished. Include a per-instance discriminator (for example module_dir_name or another unique input) in these sync key names to prevent cross-module orchestration races.
Useful? React with 👍 / 👎.
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. It should help simplify the individual agent scripts, making them way more ergonomic to maintain. Thanks, Jay.
| variable "install_script" { | ||
| type = string | ||
| description = "Script to install the agent used by AgentAPI." | ||
| default = "" |
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.
| default = "" | |
| default = null |
Description
The Agent Helper module is a building block for modules that need to run multiple scripts in a specific order. It uses
coder exp syncfor dependency management and is designed for orchestrating pre-install, install, post-install, and start scripts.Type of Change
Module Information
Path:
registry/coder/modules/agent-helperNew version:
v1.0.0Breaking change: [x] Yes [ ] No
Testing & Validation
bun test)bun fmt)Related Issues
Closes: #696
Closes: #698