Skip to content

Comments

fix: remove is_build_time from env_vars create action#101

Merged
StuMason merged 1 commit intomainfrom
fix/env-vars-is-build-time
Jan 27, 2026
Merged

fix: remove is_build_time from env_vars create action#101
StuMason merged 1 commit intomainfrom
fix/env-vars-is-build-time

Conversation

@StuMason
Copy link
Owner

Summary

Remove is_build_time parameter from the env_vars tool schema. Coolify API rejects this field for env var creation despite it being documented in the OpenAPI spec.

Changes

  • Remove is_build_time from tool schema (was misleading users)
  • Remove from create action handler
  • Add comment explaining why

Test plan

  • npm test - 219 tests passing
  • Verified against issue reproduction steps

Fixes #97

Coolify API rejects is_build_time parameter for env var creation
despite it being documented in the OpenAPI spec. Remove it from
the schema to avoid misleading users.

Fixes #97
@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.93%. Comparing base (065a667) to head (ed9785b).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #101   +/-   ##
=======================================
  Coverage   90.93%   90.93%           
=======================================
  Files           1        1           
  Lines         375      375           
  Branches      100      100           
=======================================
  Hits          341      341           
  Misses          6        6           
  Partials       28       28           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@claude
Copy link

claude bot commented Jan 26, 2026

PR Review: fix: remove is_build_time from env_vars create action

Summary

This PR addresses issue #97 where the Coolify API rejects the is_build_time parameter for env var creation, despite it being documented in the OpenAPI spec. The fix removes the parameter from the MCP tool schema for the env_vars tool's create action.


✅ Code Quality and Best Practices

Good:

  • The fix is minimal and focused - only removes what's necessary
  • Clear comment explaining why is_build_time is not passed (line 837)
  • CHANGELOG entry follows the project's Keep a Changelog format
  • PR description includes clear summary, changes, and test plan

Minor suggestion:
The comment could reference the issue number for future maintainability:

// Note: is_build_time is not passed - Coolify API rejects it for create (see #97)

✅ Correctness

The fix is correct. Looking at the codebase:

  1. The CreateEnvVarRequest type in src/types/coolify.ts:417 still has is_build_time?: boolean - this is fine since it's optional and the client method can still accept it for other callers
  2. The updateApplicationEnvVar at line 842 doesn't pass is_build_time either, which is consistent
  3. The bulkEnvUpdate tool (line 1167-1170) still accepts is_build_time - this may need investigation if it also fails on the Coolify API

Question: Does the update action also reject is_build_time? The current code doesn't pass it for update either, but the test at coolify-client.test.ts:3009 shows updating with is_build_time: true. If update accepts it but create doesn't, you might want to keep the parameter in the schema but only use it for updates.


✅ No Security Concerns

The change removes a parameter - no new attack surface introduced.


⚠️ Test Coverage Considerations

The PR mentions 219 tests passing, but I noticed:

  1. There's no direct MCP tool-level test for the env_vars tool with action: create in the test files
  2. The existing coolify-client.test.ts test at line 1227-1230 still tests createApplicationEnvVar with is_build_time: true - this is fine since the client method still accepts it, but you might want to add a test case without is_build_time to verify the new calling pattern works

Since the client method signature didn't change (it still accepts optional is_build_time), existing coverage should be sufficient. The change is in how the MCP tool calls the client, not the client itself.


✅ Performance

No performance impact - this is a simple parameter removal.


📋 Documentation

CHANGELOG entry is well-formatted and appropriately placed under "Fixed" in the Unreleased section.


Overall Assessment: Approve

This is a clean, minimal fix for a real API incompatibility issue. The code changes are correct and well-documented. The only minor suggestion is to consider whether the bulk_env_update tool might have the same issue and whether update action could benefit from having is_build_time available (if the API accepts it for updates).

Nice work tracking down this API vs OpenAPI spec discrepancy!

@StuMason StuMason merged commit a46faff into main Jan 27, 2026
7 checks passed
@StuMason StuMason deleted the fix/env-vars-is-build-time branch January 27, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

env_vars tool: is_build_time parameter causes validation error

1 participant