feat(registry/coder/modules/coder-utils): make install_script and start_script optional

[35C4n0r]

Description

Rework the coder-utils module to make start_script optional while keeping install_script required. Adds mkdir -p to every script block, updates the module directory path to a namespaced location, moves all sync dependency resolution into locals, and bumps Terraform to >= 1.9.

Type of Change

• Feature/enhancement

Module Information

Path: registry/coder/modules/coder-utils
New version: v1.1.0
Breaking change: [x] Yes [ ] No

install_script is now required (previously had default = null). Callers that omitted it will need to provide a value. start_script now defaults to null (previously required). The module directory path changed from $HOME/<name> to $HOME/.coder-modules/coder/coder-utils/<name>.

Testing & Validation

• Tests pass (terraform test — 7 passed, 0 failed)
• Code formatted
• Changes tested locally

Changes

main.tf

• install_script: now required (no default). Resource always created (no count).
• start_script: added default = null. Resource conditionally created with count.
• All four script blocks now include mkdir -p to ensure the module directory exists.
• module_dir_path updated to $HOME/.coder-modules/coder/coder-utils/<module_dir_name>.
• Sync deps moved to locals (install_sync_deps, start_sync_deps). Post-install always waits for install. Start waits for install (+ post_install when present). Install waits for pre_install when present.
• install_script_name output is always set. Other outputs return "" when variable is not provided.
• Required Terraform version bumped to >= 1.9.

main.tftest.hcl

• Updated for required install_script and optional start_script.
• Added test_install_syncs_with_pre_install and test_start_syncs_with_post_install to verify sync dependency chains.

main.test.ts

• install_script added back to required variables.

README.md

• Version bumped to 1.1.0.
• Execution order updated: install is required, start is optional.

Related Issues

None

---

Generated with Coder Agents

[Copilot]

Pull request overview

Makes install_script and start_script optional in the registry/coder/modules/coder-utils Terraform module, aligning behavior with the existing optional pre/post install scripts and updating tests accordingly.

Changes:

• Make start_script optional (adds default = null) and gate install_script/start_script resources behind count.
• Update sync dependency resolution to handle missing stages in the pre→install→post→start chain.
• Update Terraform/Bun tests to reflect count-based resources and nullable outputs, adding additional test scenarios.

Reviewed changes

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

File	Description
registry/coder/modules/coder-utils/main.tf	Makes install/start scripts optional, adds sync-dependency locals, and makes outputs nullable.
registry/coder/modules/coder-utils/main.tftest.hcl	Updates tests for count-based resources and adds new test scenarios for missing scripts.
registry/coder/modules/coder-utils/main.test.ts	Updates required-variable test to no longer require start_script.

Comments suppressed due to low confidence (1)

registry/coder/modules/coder-utils/main.tf:188

• start_script is now optional and can be the only script provided, but this script doesn't create ${local.module_dir_path} before writing ${local.start_path} / ${local.start_log_path}. In a start-only configuration (which is now explicitly supported by tests), the script will fail at runtime because the directory doesn't exist. Add mkdir -p ${local.module_dir_path} near the top of this script (or otherwise ensure the directory exists) so start-only works end-to-end.

  script       = <<-EOT
    #!/bin/bash
    set -o errexit
    set -o pipefail

    trap 'coder exp sync complete ${local.start_script_name}' EXIT

    %{if local.start_sync_deps != null~}
    coder exp sync want ${local.start_script_name} ${local.start_sync_deps}
    %{endif~}
    coder exp sync start ${local.start_script_name}

    echo -n '${local.encoded_start_script}' | base64 -d > ${local.start_path}
    chmod +x ${local.start_path}

[matifali]

Deep Review

The core goal — making install_script and start_script optional via count gating, matching the existing pattern for pre_install_script / post_install_script — is sound. The sync dependency chain logic (post_install_sync_deps, start_sync_deps) is correct and handles all combinations gracefully. Tests are thorough with good coverage of edge cases.

A few things to address before merging:

---

1. Undocumented breaking change: module_dir_path moved

This is the most significant concern. The PR silently changes module_dir_path from:

module_dir_path = "$HOME/${var.module_dir_name}"

to:

module_dir_path = "$HOME/.coder-modules/coder/coder-utils/${var.module_dir_name}"

This changes the on-disk path for all scripts, logs, and artifacts. It's not mentioned anywhere in the PR description. If intentional, it should be called out explicitly and justified. If not, it should be reverted from this PR and handled separately.

---

2. PR description says outputs return null — they return ""

The description states:

"All four outputs now return null when the corresponding variable is not provided."

But the actual code (and all test assertions) return "" (empty string). Looks like this was fixed after the Copilot review but the description wasn't updated.

---

3. Missing newline at end of main.tf

The diff ends with \ No newline at end of file. Trivial fix.

---

4. Versioning: 1.1.0 but marked as breaking

The README bumps the version to 1.1.0, but the PR description checks [x] Yes for breaking change. If it's truly breaking (and the module_dir_path change makes it so), semver convention would be 2.0.0. If the argument is that existing callers who provide these variables are unaffected, then uncheck the breaking change box.

---

5. Sync dependency chain — verified correct

I traced through all combinations:

pre_install	install	post_install	start	post_install_sync_deps	start_sync_deps
Y	Y	Y	Y	install	install + post_install
N	Y	N	Y	—	install
Y	N	Y	Y	pre_install	post_install
N	N	N	Y	—	null
Y	N	N	Y	—	pre_install

All correct. The chain degrades gracefully when any middle step is absent.

---

6. mkdir -p added to post_install_script and start_script — good

With install_script now optional, the directory may not exist when later scripts run. The mkdir -p addition is the right fix.

---

7. Minor: test coverage gap

No test for post_install_only (just post_install_script, no other scripts). This would verify post_install_sync_deps resolves to null when both pre_install_script and install_script are absent.

---

Area	Status
Core optionality logic	✅ Good
Sync dependency chain	✅ Correct
Test coverage	✅ Good (minor gap)
mkdir -p fix	✅ Good
module_dir_path change	⚠️ Undocumented — needs discussion
PR description accuracy	⚠️ Needs update (null → "")
Version bump vs breaking flag	⚠️ Inconsistent
Missing newline at EOF	🔧 Trivial fix

🤖 This response was generated by Coder Agents.

[DevelopmentCats]

Ultimately this is a NIT and not a blocker.

I know we were debating on where we actually wanted all of this to live but did we finally decide that this is what we wanted over anything else?

I only ask because coder-registry/ comes to mind since we could isolate all of module related files into that one folder since they derive from here.

[matifali]

there is also CODER_SCRIPT_BIN_DIR.

[35C4n0r]

I only ask because coder-registry/ comes to mind since we could isolate all of module related files into that one folder since they derive from here.

like $HOME/coder-registry/author/module_name/* ?

also we also want this to be configurable, so I'll add a variable here as I'm planning to do with boundary, tasks , and other ai module.

locals{
    resolved_base_module_dir = var.base_module_dir ? var.base_module_dir : "$HOME/coder-registry"
    module_dir_path = "${local.resolved_base_module_dir}/author/${var.module_dir_name}"
}

@DevelopmentCats @matifali thoughts ?

[35C4n0r]

oof, that make author hardcoded, and other users won't be able to use it.

[35C4n0r]

I know we were debating on where we actually wanted all of this to live but did we finally decide that this is what we wanted over anything else?

thread