Skip to content

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

Merged
merged 21 commits into from
Aug 8, 2025
Merged

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Aug 5, 2025

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 running main.sh in agentapi, so this environment variable is available to calling modules if var.subdomain = false.

Type of Change

  • New module
  • Bug fix
  • Feature/enhancement
  • Documentation
  • Other

Testing & Validation

  • Tests pass (bun test)
  • Code formatted (bun run fmt)
  • Changes tested locally

Related Issues

coder/coder#18779

@johnstcn johnstcn self-assigned this Aug 5, 2025
@johnstcn johnstcn requested a review from Copilot August 5, 2025 14:59
Copy link

@Copilot Copilot AI left a 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

@hugodutka
Copy link
Contributor

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 main.sh in main.tf, and all dependent modules would pick it up automatically. What do you think @johnstcn? I was planning to add that feature anyway.

@johnstcn johnstcn force-pushed the cj/goose/subpath branch 3 times, most recently from 75dab94 to be1a23b Compare August 5, 2025 19:06
@johnstcn johnstcn requested review from matifali and hugodutka August 5, 2025 19:09
Copy link
Contributor

@DevelopmentCats DevelopmentCats left a 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?

@matifali matifali added the version:minor Add to PRs requiring a minor version upgrade label Aug 6, 2025

This comment was marked as outdated.

@johnstcn
Copy link
Member Author

johnstcn commented Aug 6, 2025

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 main.sh in main.tf, and all dependent modules would pick it up automatically. What do you think @johnstcn? I was planning to add that feature anyway.

Done

@johnstcn
Copy link
Member Author

johnstcn commented Aug 6, 2025

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?

Done!

Base automatically changed from cj/agentapi/subpath to main August 6, 2025 11:38
@DevelopmentCats DevelopmentCats merged commit 74c8698 into main Aug 8, 2025
4 checks passed
@DevelopmentCats DevelopmentCats deleted the cj/goose/subpath branch August 8, 2025 03:12
@matifali matifali mentioned this pull request Aug 8, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version:minor Add to PRs requiring a minor version upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants