-
Notifications
You must be signed in to change notification settings - Fork 43
feat: goose: add support for subdomain=false #299
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
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 adds support for specifying subdomain = false
in the goose module to allow AgentAPI to be accessed without using a subdomain. This builds on changes in the coder/registry repository and addresses issue #18779.
- Added a new
subdomain
variable to control whether AgentAPI uses a subdomain - Updated the start script to conditionally include the
--chat-base-path
parameter when subdomain is disabled - Added test coverage for the new subdomain=false functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
registry/coder/modules/goose/main.tf | Added subdomain variable and updated agentapi module version with subdomain parameter |
registry/coder/modules/goose/scripts/start.sh | Modified script to conditionally add chat-base-path argument based on environment variable |
registry/coder/modules/goose/main.test.ts | Added test case to verify subdomain=false functionality works correctly |
Hmm, instead of passing the base path as a command line argument, why don't we update AgentAPI to support configuration via environment variables? Then you'd only need to specify the chat base path before running |
… AGENTAPI_CHAT_BASE_PATH will land
75dab94
to
be1a23b
Compare
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 since we have addressed the above comments.
Outside of waiting for #297 to be approved, Can you bump the module version for goose with a patch bump as well?
This comment was marked as outdated.
This comment was marked as outdated.
Done |
Done! |
23fd2ce
to
f0a675e
Compare
Updates coder/coder#18779
Builds on #297
Description
Adds support for specifying
subdomain = false
in the agentapi module.Change added in #297
NOTE:
AGENTAPI_CHAT_BASE_PATH
is exported before runningmain.sh
in agentapi, so this environment variable is available to calling modules ifvar.subdomain = false
.Type of Change
Testing & Validation
bun test
)bun run fmt
)Related Issues
coder/coder#18779