Skip to content

fix(deploy): skip dead service rows in preflight, scope migration compose project - #327

Merged
Hydralerne merged 2 commits into
oblien:mainfrom
DiogoDuart3:fix/preflight-dead-service-rows
Jul 31, 2026
Merged

fix(deploy): skip dead service rows in preflight, scope migration compose project#327
Hydralerne merged 2 commits into
oblien:mainfrom
DiogoDuart3:fix/preflight-dead-service-rows

Conversation

@DiogoDuart3

Copy link
Copy Markdown
Contributor

What / why

Two related deploy/migration robustness fixes.

1. Preflight hard-fails a whole project over one dead service row

The multi-service preflight's sub-app check hard-fails the entire
project's
deploy when any enabled monorepo-kind service row lacks an
install/build/start command it's supposed to have. That's the right call
for a genuinely misconfigured, actively-used sub-app — but it applies just
as unconditionally to a saved row that has never once been deployed: a
duplicate/leftover row from an earlier import, or a half-finished "add
service" that was never followed through.

Hit this on a real self-hosted install: a docker-migration-imported project
accumulated a stray, never-deployed monorepo-kind service row alongside
its real (already-working, already-routed) compose services. That one dead
row's missing start command blocked every subsequent deploy of the whole
project indefinitely
— including a plain git-push redeploy that touched
none of the project's actual services — with no way to recover short of an
operator finding and editing the row directly in the database.

Fix: track whether a service has ever produced a service_deployment
row (one extra round trip via the existing serviceDeployment.latestByProject
lookup — no new query shape, just reusing what's already there). A saved
row with no install/build/start command and no deployment history is
downgraded from a hard failure to a warning: the operator still sees it
flagged, but the rest of the project's services keep deploying. A row that
has been deployed before, or one that came from an explicit in-flight
request (not projected from the DB, so its history is unknown), is
unaffected and still fails exactly as before.

2. Adopted service-name resolution isn't scoped to a compose project

adoptServerStack() resolved the caller's serviceNames against every
service discovered on the server, server-wide. Service names are only
unique within a single compose project, so a server running several
stacks has a bare name like app/db/redis matching a container in
each of them — and those extra cross-stack matches weren't dropped:
buildAdoptedServiceRows suffixed them (app-2, redis-3) and silently
adopted another project's containers into the one being migrated.

Fix: accept an optional composeProject to restrict resolution to ONE
discovered group (the compose project name, or null for the standalone/
hand-run-container group). Omitting it keeps the legacy server-wide match
for existing callers, so this is purely additive.

Test plan

  • tsc --noEmit on apps/api — clean.
  • Added two tests to apps/api/test/modules/deployments/preflight.test.ts:
    one confirming a never-deployed, commandless monorepo row now produces a
    warn (and result.ok === true), one confirming a row that has been
    deployed before still hard-fails exactly as before (no regression).
  • No test hunk existed for the composeProject migration change; the
    existing migrate.service.test.ts suite is unaffected since the new
    parameter is optional and defaults to the prior server-wide behavior.
  • Note: this sandbox's vitest run hits a pre-existing, unrelated failure
    (TypeError: undefined is not an object (evaluating '__vite_ssr_import_0__.z.object') in packages/core/src/apps/schema.ts)
    affecting every test file in apps/api/packages/adapters identically on
    a clean origin/main checkout with zero changes (confirmed via git stash), so it isn't introduced by this PR. I traced runPreflightChecks's
    aggregation (ok: checks.every(check => check.status !== "fail")) by hand
    to confirm a warn-only result correctly leaves ok: true.

…hard-failing

The multi-service preflight's sub-app check hard-fails the ENTIRE project's
deploy when any enabled monorepo-kind service row lacks an install/build/
start command it's supposed to have. That's correct for a genuinely
misconfigured, actively-used sub-app — but it applies just as unconditionally
to a saved row that has never once been deployed: a duplicate/leftover row
from an earlier import, or a half-finished "add service" that was never
followed through.

Hit this on a real self-hosted install: a docker-migration-imported project
accumulated a stray, never-deployed monorepo-kind service row alongside its
real (already-working, already-routed) compose services. That one dead row's
missing start command blocked EVERY subsequent deploy of the whole project
indefinitely — including a plain git-push redeploy that touched none of the
project's actual services — with no way to recover short of an operator
finding and editing the row directly in the database.

Fix: track whether a service has ever produced a service_deployment row
(one extra round trip via the existing `serviceDeployment.latestByProject`
lookup — no new query shape). A saved row with NO install/build/start
command AND no deployment history is downgraded from a hard failure to a
warning: the operator still sees it flagged, but the rest of the project's
services keep deploying. A row that HAS been deployed before, or that came
from an explicit in-flight request (not projected from the DB, so its
history is unknown), is unaffected and still fails exactly as before.
…project

adoptServerStack() resolved the caller's serviceNames against every service
discovered on the server, server-wide - service names are only unique WITHIN
a single compose project, so a server running several stacks has a bare name
like "app"/"db"/"redis" matching a container in EACH of them.

Those extra cross-stack matches were not dropped: buildAdoptedServiceRows
suffixed them ("app-2", "redis-3") and silently adopted another project's
containers into the one being migrated.

Fix: accept an optional composeProject to restrict resolution to ONE
discovered group (the compose project name, or null for the standalone/
hand-run-container group). Omitting it keeps the legacy server-wide match
for existing callers.
@DiogoDuart3

Copy link
Copy Markdown
Contributor Author

The failing Test check here is a pre-existing failure on main's own tip (currently 6a677363, e.g. run 30536159910) — @repo/dashboard#test fails identically with zero changes from this PR. None of this PR's files touch apps/dashboard.

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.

2 participants