feat(resolve): warn that apply cannot enable "Include Source Commit in Build" (#46) #54

Merged

dan-claude-bot merged 1 commit from feat/source-commit-notice into main

2026-07-14 22:42:30 +00:00

dan-claude-bot commented

2026-07-14 22:24:42 +00:00

(Migrated from github.com)

The question

Can "Include Source Commit in Build" be set via the Coolify API? If it can, apply should set it. If it can't, apply should say so.

The answer: it cannot. Taking the second branch.

I have no Coolify credentials, so I could not run the live curl probe from the issue — but the probe was unnecessary, and a live probe would have been weaker evidence than what follows: it proves the answer for one box, whereas the source proves it for every box. I read the whole v4.1.2 tagged tree and verified each claim independently. Full write-up posted to the issue: #46 (comment) → https://github.com/heavy-duty/cast/issues/46#issuecomment-4974553276

1. The field is an application setting, defaulting to off.

app/Models/ApplicationSetting.php l.19 (boolean cast), l.66 (fillable)
database/migrations/2025_11_26_124200_…php l.22 — ->default(false), so every app cast creates has it off.

2. It has zero API surface. A whole-tree grep (excluding vendor/) for include_source_commit_in_build|includeSourceCommitInBuild returns 17 hits, none under app/Http/Controllers/Api/. The only writer in the entire codebase:

// app/Livewire/Project/Application/Advanced.php:128
$this->application->settings->include_source_commit_in_build = $this->includeSourceCommitInBuild;

A Livewire component — a human, in the Advanced tab.

3. And the API would reject it, not ignore it — this is what rules out "send it anyway and hope":

// app/Http/Controllers/Api/ApplicationsController.php:2430-2436  (update_by_uuid → PATCH /applications/{uuid})
$extraFields = array_diff(array_keys($request->all()), $allowedFields);
if ($validator->fails() || ! empty($extraFields)) {
    foreach ($extraFields as $field) { $errors->add($field, 'This field is not allowed.'); }
    return response()->json(['message' => 'Validation failed.', ...

PATCH allowlist (l.2368): 71 fields — not among them.
Create allowlist (l.914): 81 fields — not among them either, so it cannot be set at create time or after.
Both allowlists do contain connect_to_docker_network — which is exactly why that create-time field works today (cli.ts:2054), and is the perfect control for this experiment.

Minor correction to #50: the PATCH allowlist is 71 fields, not 68. Immaterial to the conclusion; flagged so the number isn't re-quoted wrong.

⚠️ #46's original premise was wrong — and #50 found the real cause

Worth stating loudly, because it changes what the warning should say. The toggle gates only the build-time arg:

// app/Jobs/ApplicationDeploymentJob.php:2949  (identically at 2993)
if (! $forBuildTime || $this->application->settings->include_source_commit_in_build) {

At runtime $forBuildTime is false → the condition short-circuits true regardless of the toggle. Coolify's runtime injection of SOURCE_COMMIT is unconditional. A service reading process.env.SOURCE_COMMIT at request time — exactly what /version does — never needed this toggle.

So the toggle was not why a live box reported {"sha":"unknown"}. The real cause is the very next line (:2950): Coolify skips its own injection if the app already carries an env var of that name, so an orphan (even empty) app-level SOURCE_COMMIT suppresses it. That is #50, in flight in parallel. Not implemented here.

Consequently I did not ship the line drafted in the issue (…or /version will report sha "unknown") — that sentence is false, and baking a wrong premise into the tool's own permanent output would mislead every operator who later reads it. The shipped wording states only what the source proves.

What changed

Three files, small diff.

src/resolve.ts — desiredFromManifest now warns once per dockercompose application, using the same mechanism and voice as the existing umami service-domains warning:

application core builds with dockercompose, but apply cannot enable "Include Source
Commit in Build" on Coolify 4.1.2 — the setting is absent from the API's field
allowlist. If the build consumes SOURCE_COMMIT as a build arg, enable it in the
Coolify UI and redeploy; Coolify injects SOURCE_COMMIT at runtime regardless.

The invariant is pinned in a comment placed exactly where a future reader would otherwise "fix" this by adding include_source_commit_in_build to fields — which would 422 every apply, since the allowlist rejects unknown keys. The comment says so, and says why connect_to_docker_network is not a precedent for it.

test/resolve.test.ts — two new tests: the warning fires for a dockercompose app (and names the app, the setting, and the UI), and does not fire for a non-compose app. Plus a regression guard that include_source_commit_in_build never reaches fields. Also added a console.warn spy to the pre-existing dockercompose test to keep test output clean.

docs/semantics.md — recorded under Known limitations, not defects, with the citations, the connect_to_docker_network contrast, and the build-time/runtime distinction.

Verification

npm run check && npm run build && npm test — 296/296 green (294 baseline + 2 new). Also drove the built artifact against a real dockercompose manifest to confirm the warning actually reaches stderr and that the unsettable field stays out of the payload.

Noted, not fixed (staying in scope)

The schema requires service_domains on every dockercompose app, so a compose app with no exposed HTTP service still has to declare one. Not touched.

Closes #46.

🤖 Generated with Claude Code

## The question > Can *"Include Source Commit in Build"* be set via the Coolify API? **If it can**, `apply` should set it. **If it can't**, `apply` should say so. ## The answer: it **cannot**. Taking the second branch. I have no Coolify credentials, so I could not run the live `curl` probe from the issue — but the probe was unnecessary, and a live probe would have been *weaker* evidence than what follows: it proves the answer for one box, whereas the source proves it for every box. I read the whole **v4.1.2** tagged tree and verified each claim independently. Full write-up posted to the issue: **#46 (comment)** → https://github.com/heavy-duty/cast/issues/46#issuecomment-4974553276 **1. The field is an application *setting*, defaulting to off.** - `app/Models/ApplicationSetting.php` **l.19** (boolean cast), **l.66** (fillable) - `database/migrations/2025_11_26_124200_…php` **l.22** — `->default(false)`, so every app cast creates has it off. **2. It has zero API surface.** A whole-tree grep (excluding `vendor/`) for `include_source_commit_in_build|includeSourceCommitInBuild` returns 17 hits, **none under `app/Http/Controllers/Api/`**. The only writer in the entire codebase: ```php // app/Livewire/Project/Application/Advanced.php:128 $this->application->settings->include_source_commit_in_build = $this->includeSourceCommitInBuild; ``` A Livewire component — a human, in the Advanced tab. **3. And the API would *reject* it, not ignore it** — this is what rules out "send it anyway and hope": ```php // app/Http/Controllers/Api/ApplicationsController.php:2430-2436 (update_by_uuid → PATCH /applications/{uuid}) $extraFields = array_diff(array_keys($request->all()), $allowedFields); if ($validator->fails() || ! empty($extraFields)) { foreach ($extraFields as $field) { $errors->add($field, 'This field is not allowed.'); } return response()->json(['message' => 'Validation failed.', ... ``` - PATCH allowlist (**l.2368**): **71** fields — not among them. - Create allowlist (**l.914**): **81** fields — not among them either, so it cannot be set at create time *or* after. - Both allowlists **do** contain `connect_to_docker_network` — which is exactly why *that* create-time field works today (`cli.ts:2054`), and is the perfect control for this experiment. > Minor correction to #50: the PATCH allowlist is **71** fields, not 68. Immaterial to the conclusion; flagged so the number isn't re-quoted wrong. ## ⚠️ #46's original premise was wrong — and #50 found the real cause Worth stating loudly, because it changes what the warning should *say*. The toggle gates **only the build-time arg**: ```php // app/Jobs/ApplicationDeploymentJob.php:2949 (identically at 2993) if (! $forBuildTime || $this->application->settings->include_source_commit_in_build) { ``` At runtime `$forBuildTime` is `false` → the condition **short-circuits true regardless of the toggle**. **Coolify's runtime injection of `SOURCE_COMMIT` is unconditional.** A service reading `process.env.SOURCE_COMMIT` at request time — exactly what `/version` does — **never needed this toggle.** So the toggle was **not** why a live box reported `{"sha":"unknown"}`. The real cause is the very next line (`:2950`): Coolify **skips its own injection if the app already carries an env var of that name**, so an orphan (even empty) app-level `SOURCE_COMMIT` *suppresses* it. That is **#50**, in flight in parallel. **Not implemented here.** Consequently I did **not** ship the line drafted in the issue (`…or /version will report sha "unknown"`) — that sentence is false, and baking a wrong premise into the tool's own permanent output would mislead every operator who later reads it. The shipped wording states only what the source proves. ## What changed Three files, small diff. **`src/resolve.ts`** — `desiredFromManifest` now warns once per `dockercompose` application, using the same mechanism and voice as the existing umami service-domains warning: ``` application core builds with dockercompose, but apply cannot enable "Include Source Commit in Build" on Coolify 4.1.2 — the setting is absent from the API's field allowlist. If the build consumes SOURCE_COMMIT as a build arg, enable it in the Coolify UI and redeploy; Coolify injects SOURCE_COMMIT at runtime regardless. ``` The invariant is pinned in a comment placed exactly where a future reader would otherwise "fix" this by adding `include_source_commit_in_build` to `fields` — which would 422 **every** apply, since the allowlist rejects unknown keys. The comment says so, and says why `connect_to_docker_network` is not a precedent for it. **`test/resolve.test.ts`** — two new tests: the warning fires for a dockercompose app (and names the app, the setting, and the UI), and does **not** fire for a non-compose app. Plus a regression guard that `include_source_commit_in_build` never reaches `fields`. Also added a `console.warn` spy to the pre-existing dockercompose test to keep test output clean. **`docs/semantics.md`** — recorded under *Known limitations, not defects*, with the citations, the `connect_to_docker_network` contrast, and the build-time/runtime distinction. ## Verification `npm run check && npm run build && npm test` — **296/296 green** (294 baseline + 2 new). Also drove the built artifact against a real dockercompose manifest to confirm the warning actually reaches stderr and that the unsettable field stays out of the payload. ## Noted, not fixed (staying in scope) - The schema requires `service_domains` on every dockercompose app, so a compose app with no exposed HTTP service still has to declare one. Not touched. Closes #46. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

No reviewers

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/cast#54

No description provided.