Skip to content

Conversation

DevelopmentCats
Copy link
Contributor

@DevelopmentCats DevelopmentCats commented Aug 25, 2025

Closes #

Description

  • Introduces subdomain variable
  • Logic for subdomain and base path

Tested with and without subdomain to ensure no breaking changes

Type of Change

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

Module Information

Path: registry/coder/modules/claude-code
New version: v2.2.0
Breaking change: [X] Yes [ ] No

Testing & Validation

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

Related Issues

@Copilot Copilot AI review requested due to automatic review settings August 25, 2025 20:47
@DevelopmentCats DevelopmentCats self-assigned this Aug 25, 2025
Copy link
Contributor

@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 introduces subdomain routing support for the Claude Code module, allowing users to choose between subdomain and path-based routing. The changes enhance the module's flexibility while maintaining backward compatibility.

  • Added subdomain variable to control routing method (subdomain vs. path-based)
  • Updated AgentAPI version from v0.3.0 to v0.3.3 to support required environment variable configuration
  • Implemented conditional logic for base paths and environment variables based on subdomain setting

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
main.tf Added subdomain variable, conditional path logic, and updated AgentAPI version
README.md Updated version references from 2.1.0 to 2.2.0

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@hugodutka hugodutka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic looks good to me, but it's a shame it's getting duplicated here - it's already present in the agentapi module.

It's high time we migrated the claude code module to use the agentapi module - the divergence is getting hard to manage.

I haven't tested this locally, so please make sure it works before merging.

@matifali
Copy link
Member

It's on list the list @hugodutka.
@35C4n0r will be doing that next.

@DevelopmentCats
Copy link
Contributor Author

The logic looks good to me, but it's a shame it's getting duplicated here - it's already present in the agentapi module.

It's high time we migrated the claude code module to use the agentapi module - the divergence is getting hard to manage.

I haven't tested this locally, so please make sure it works before merging.

I tested this locally with and without the subdomain option to verify it works and that it doesn't break any existing templates, and it was working perfectly.

@matifali
Copy link
Member

@DevelopmentCats Lets merge and unblock @greg-the-coder

@matifali matifali added the version:minor Add to PRs requiring a minor version upgrade label Aug 26, 2025
@DevelopmentCats DevelopmentCats merged commit c8441fc into main Aug 27, 2025
5 checks passed
@DevelopmentCats DevelopmentCats deleted the cat/claude-subdomain branch August 27, 2025 01:36
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.

3 participants