fix: resolve the GitHub App only when the manifest declares applications (#103) #107

Merged

dan-claude-bot merged 1 commit from fix/lazy-github-app into main

2026-07-19 12:58:02 +00:00

dan-claude-bot commented

2026-07-19 12:26:50 +00:00

(Migrated from github.com)

Fixes #103.

What

Found live in the 2026-07-19 release drill against two real Coolify 4.1.2 instances: a manifest declaring only databases (applications: {}) rendered its plan (2 change(s)) and then died in preflight with no GitHub App bound for dan-claude-bot/drill-widget. runProject resolved the server AND the GitHub App unconditionally before building the executor, so githubAppNameFor threw over a binding nothing in the run would ever have used — a GitHub App is read by exactly one call, the application create (POST /applications/private-github-app); databases and services never touch it. That gated infra-only projects (databases/services shared by a fleet's other projects) behind the GitHub-App browser-registration ceremony.

How

runProject resolves the App — binding lookup and client.githubAppUuid both — only when the desired state contains at least one application, and passes null through otherwise.
The executor's githubAppUuid field is typed string | null; its single consumer (the application create) guards the null with cast's own internal error. Unreachable by construction — a plan can only create resources the desired state holds — but a null slipping onto the wire would otherwise surface as a Coolify 422 about somebody else's field.
Keyed off desired, not the plan's changes: a manifest that declares an application keeps the missing-binding refusal even on a clean plan, byte-identical to before — that binding is state the next create will need.

Tests

test/github-app-cli.test.ts (new, end to end against a stub Coolify): a databases-only manifest with github_apps: {} applies all the way through (applied + redeployed: cache) and the stub never sees a /github-apps request; a manifest WITH an application still refuses on the missing binding, pinning the exact no GitHub App bound message and that nothing was created.
test/wire.test.ts: a null-App executor creates a database sending no github_app_uuid, and refuses an application create with the internal error before anything reaches the create route.

Suite: 600 tests across 34 files, all passing. npm run check (biome) and npm run build clean.

🤖 Generated with Claude Code

Fixes #103. ## What Found live in the 2026-07-19 release drill against two real Coolify 4.1.2 instances: a manifest declaring only databases (`applications: {}`) rendered its plan (`2 change(s)`) and then died in preflight with `no GitHub App bound for dan-claude-bot/drill-widget`. `runProject` resolved the server AND the GitHub App unconditionally before building the executor, so `githubAppNameFor` threw over a binding nothing in the run would ever have used — a GitHub App is read by exactly one call, the application create (`POST /applications/private-github-app`); databases and services never touch it. That gated infra-only projects (databases/services shared by a fleet's other projects) behind the GitHub-App browser-registration ceremony. ## How - `runProject` resolves the App — binding lookup and `client.githubAppUuid` both — only when the desired state contains at least one application, and passes `null` through otherwise. - The executor's `githubAppUuid` field is typed `string | null`; its single consumer (the application create) guards the null with cast's own internal error. Unreachable by construction — a plan can only create resources the desired state holds — but a null slipping onto the wire would otherwise surface as a Coolify 422 about somebody else's field. - Keyed off **desired**, not the plan's changes: a manifest that declares an application keeps the missing-binding refusal even on a clean plan, byte-identical to before — that binding is state the next create will need. ## Tests - `test/github-app-cli.test.ts` (new, end to end against a stub Coolify): a databases-only manifest with `github_apps: {}` applies all the way through (`applied + redeployed: cache`) and the stub never sees a `/github-apps` request; a manifest WITH an application still refuses on the missing binding, pinning the exact `no GitHub App bound` message and that nothing was created. - `test/wire.test.ts`: a null-App executor creates a database sending no `github_app_uuid`, and refuses an application create with the internal error before anything reaches the create route. Suite: 600 tests across 34 files, all passing. `npm run check` (biome) and `npm run build` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-19 12:26:50 +00:00

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-19 12:39:07 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Reviewed — I agree with all of this, no concerns.

Keying the App resolution off desired.some((d) => d.kind === "application") rather than the plan's changes is the right choice — a manifest that declares an app keeps the missing-binding refusal even on a clean plan, since that binding is state the next create will need. Typing githubAppUuid as string | null and guarding its single consumer (the application create) with cast's own internal error is the correct belt-and-suspenders: unreachable by construction, but a null slipping onto the wire would otherwise surface as an opaque Coolify 422 about somebody else's field. Tests pin both directions end-to-end (databases-only applies and the stub never sees a /github-apps request; an app manifest still refuses on the missing binding, nothing created) plus the wire-level null-App executor. Clean.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Reviewed — I agree with all of this, no concerns.** Keying the App resolution off `desired.some((d) => d.kind === "application")` rather than the plan's changes is the right choice — a manifest that declares an app keeps the missing-binding refusal even on a clean plan, since that binding is state the next create will need. Typing `githubAppUuid` as `string | null` and guarding its single consumer (the application create) with cast's own internal error is the correct belt-and-suspenders: unreachable by construction, but a null slipping onto the wire would otherwise surface as an opaque Coolify 422 about somebody else's field. Tests pin both directions end-to-end (databases-only applies and the stub never sees a `/github-apps` request; an app manifest still refuses on the missing binding, nothing created) plus the wire-level null-App executor. Clean. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:43:56 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Approve — the laziness is keyed correctly, the refusal for app-bearing manifests is preserved exactly (clean plan or not), and both directions are pinned end-to-end. Verified against the full suite: 600/600 passing, build and biome clean.

Specifics checked:

src/cli.ts:1291-1293 — gating on desired.some((d) => d.kind === "application") covers both failure modes behind one guard: the missing-binding throw from githubAppNameFor and the not found in Coolify: github app throw from client.githubAppUuid. Both still fire in preflight, before buildExecutor and any write, so the error for app manifests is as early and as clear as before. Keying off desired rather than report.changes is the right call for the clean-plan case.
src/cli.ts:3413 is the only consumer of ctx.githubAppUuid anywhere in src/ — updates, database/service creates, and backups never read it — so string | null plus the guard at src/cli.ts:3403 is sound. The "unreachable by construction" claim holds: apply never deletes orphans, so no application-shaped change can reach the executor from an applications-free desired.
Fleet applies (--all) route through the same runProject (src/cli.ts:1466,1478), and apply --resource is refused up front, so no partial-plan path can subvert the gate.
test/github-app-cli.test.ts asserts the load-bearing absence (hits never contains /github-apps), not just exit code 0 — that pins the resolution being skipped, not merely the create. The with-application case pins the exact refusal text and that nothing was created.

No changes requested.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

**Approve** — the laziness is keyed correctly, the refusal for app-bearing manifests is preserved exactly (clean plan or not), and both directions are pinned end-to-end. Verified against the full suite: 600/600 passing, build and biome clean. Specifics checked: - `src/cli.ts:1291-1293` — gating on `desired.some((d) => d.kind === "application")` covers both failure modes behind one guard: the missing-binding throw from `githubAppNameFor` and the `not found in Coolify: github app` throw from `client.githubAppUuid`. Both still fire in preflight, before `buildExecutor` and any write, so the error for app manifests is as early and as clear as before. Keying off `desired` rather than `report.changes` is the right call for the clean-plan case. - `src/cli.ts:3413` is the only consumer of `ctx.githubAppUuid` anywhere in `src/` — updates, database/service creates, and backups never read it — so `string | null` plus the guard at `src/cli.ts:3403` is sound. The "unreachable by construction" claim holds: `apply` never deletes orphans, so no application-shaped change can reach the executor from an applications-free `desired`. - Fleet applies (`--all`) route through the same `runProject` (`src/cli.ts:1466,1478`), and `apply --resource` is refused up front, so no partial-plan path can subvert the gate. - `test/github-app-cli.test.ts` asserts the load-bearing absence (`hits` never contains `/github-apps`), not just exit code 0 — that pins the resolution being skipped, not merely the create. The with-application case pins the exact refusal text and that nothing was created. No changes requested. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:49:36 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Why

A GitHub App is only consumed on application create (POST /applications/private-github-app). Resolving it unconditionally made infra-only manifests (databases / services, applications: {}) die in preflight over a binding nothing would use — exactly the drill failure.
Keying off desired (desired.some(d => d.kind === "application")) rather than the plan's change set is the correct conservatism: a clean plan for an app-bearing manifest still refuses a missing binding, because the next create will need it.
githubAppUuid: string | null plus the internal guard at the single consumer is the right belt-and-suspenders: unreachable by construction, but a null on the wire would become a Coolify 422 about someone else's field.
E2E pins both directions (databases-only apply never hits /github-apps; app-bearing still refuses with the historical message and creates nothing). Wire tests pin null-App database create and the internal refuse on application create.

No blockers.

**Verdict: Approve** — I agree with this as-is. ### Why - A GitHub App is only consumed on application create (`POST /applications/private-github-app`). Resolving it unconditionally made infra-only manifests (databases / services, `applications: {}`) die in preflight over a binding nothing would use — exactly the drill failure. - Keying off **desired** (`desired.some(d => d.kind === "application")`) rather than the plan's change set is the correct conservatism: a clean plan for an app-bearing manifest still refuses a missing binding, because the next create will need it. - `githubAppUuid: string | null` plus the internal guard at the single consumer is the right belt-and-suspenders: unreachable by construction, but a null on the wire would become a Coolify 422 about someone else's field. - E2E pins both directions (databases-only apply never hits `/github-apps`; app-bearing still refuses with the historical message and creates nothing). Wire tests pin null-App database create and the internal refuse on application create. No blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 12:49:52 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

GitHub App resolution is now limited to desired states containing applications, while application-bearing manifests retain the original preflight refusal even on a clean plan. The nullable executor context is guarded at its sole application-create consumer, end-to-end tests cover both directions, and current checks are green.

Verdict: I agree with everything and have no additional feedback. GitHub App resolution is now limited to desired states containing applications, while application-bearing manifests retain the original preflight refusal even on a clean plan. The nullable executor context is guarded at its sole application-create consumer, end-to-end tests cover both directions, and current checks are green.

dan-claude-bot commented

2026-07-19 12:56:34 +00:00

(Migrated from github.com)

@danmt — handing this one to you. Final state:

What it fixes: cast#103 — apply resolved the GitHub App (binding lookup + client.githubAppUuid) unconditionally in preflight, even for a manifest declaring zero applications. A databases-only project rendered its plan and then died on no GitHub App bound — in the drill this blocked the whole A→B promotion until a placeholder App row was seeded on both instances, pure ceremony for a resource the apply never touches.

The fix: resolution is keyed off the desired state — desired.some(d => d.kind === "application") — not the plan's changes, so a manifest that declares an app keeps the missing-binding refusal even on a clean plan (that binding is state the next create will need). The executor's githubAppUuid is retyped string | null, with the null guarded at its single consumer (the application-create POST) by an internal error, unreachable by construction — a null slipping onto the wire would otherwise surface as an opaque Coolify 422.

Review history: one round — all three bots approved, no changes requested; claude-bot specifically endorsed keying off desired state rather than the plan.

Verification: npm test 600/0 · biome clean. End-to-end against the stub: databases-only manifest applies with zero /github-apps requests observed; an app-declaring manifest still refuses word-for-word with nothing created; wire-level tests pin the null-typed executor.

No open questions from the round. Ready for your review.

🤖 Generated with Claude Code

@danmt — handing this one to you. Final state: **What it fixes:** cast#103 — `apply` resolved the GitHub App (binding lookup + `client.githubAppUuid`) unconditionally in preflight, even for a manifest declaring zero applications. A databases-only project rendered its plan and then died on `no GitHub App bound` — in the drill this blocked the whole A→B promotion until a placeholder App row was seeded on both instances, pure ceremony for a resource the apply never touches. **The fix:** resolution is keyed off the desired state — `desired.some(d => d.kind === "application")` — not the plan's changes, so a manifest that declares an app keeps the missing-binding refusal even on a clean plan (that binding is state the next create will need). The executor's `githubAppUuid` is retyped `string | null`, with the null guarded at its single consumer (the application-create POST) by an internal error, unreachable by construction — a null slipping onto the wire would otherwise surface as an opaque Coolify 422. **Review history:** one round — all three bots approved, no changes requested; claude-bot specifically endorsed keying off desired state rather than the plan. **Verification:** `npm test` 600/0 · biome clean. End-to-end against the stub: databases-only manifest applies with zero `/github-apps` requests observed; an app-declaring manifest still refuses word-for-word with nothing created; wire-level tests pin the null-typed executor. No open questions from the round. Ready for your review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)