Skip to content

refactor: remove dead legacy DB-config code paths (phase 1)#228

Open
vigneshrajsb wants to merge 2 commits into
mainfrom
cleanup/pr-b-remove-bridge
Open

refactor: remove dead legacy DB-config code paths (phase 1)#228
vigneshrajsb wants to merge 2 commits into
mainfrom
cleanup/pr-b-remove-bridge

Conversation

@vigneshrajsb

Copy link
Copy Markdown
Contributor

Summary

Phase 1 of a two-phase cleanup that removes the dead legacy database-config code paths left over from Lifecycle's migration to YAML-based configuration. Every environment is now created with enableFullYaml = true, classicModeOnly has no writers, and per-service config lives in lifecycle.yaml pulled at build time — so the DB-config branches are unreachable.

This PR contains two logically-separate commits, reviewable in sequence:

1. refactor: collapse enableFullYaml and classicModeOnly dead code paths

Collapses ~137 conditional sites across 19 files to the YAML/deployable path:

  • Every build.enableFullYaml ? deploy.deployable.X : deploy.service.X now reads deploy.deployable.X.
  • Removes flag writes, the enableFullYamlSupport getter, the queue-fingerprint field, and the flag from API responses/swagger.
  • Deletes classic-only helpers (getClassicServiceGroups, getClassicServiceOverrideState, generateResourceRequests/LimitsForService, the configurations-table read path).
  • Model field declarations removed; DB columns untouched (phase 2).

2. refactor: remove DB->deployable bridge and legacy Deploy.service pointer

Makes deployables purely YAML-sourced:

  • Deletes the DB→deployable bridge (updateOrCreateDeployableUsingDbConfig and friends). A lifecycle.yaml defaultServices/optionalServices entry carrying serviceId now warn-and-skips instead of resolving a DB Service row.
  • Deletes setupDefaultBuildServiceOverrides/createBuildServiceOverride, ServiceService, and the BuildServiceOverride model.
  • Removes the Deploy.service relation + serviceId field, Build.buildServiceOverrides, and Environment.services/defaultServices/optionalServices relations — stripping the service token from every graph-fetch string across the agent/session/build/deploy code.
  • Updates the query_database AI-tool relation allowlist to drop the removed deploys.service relation.

Not in scope (phase 2)

DB tables/columns stay until this deploys cleanly and is verified in production. Phase 2 then drops services, configurations, services_disks, environmentDefaultServices/environmentOptionalServices, build_service_overrides, the flag columns, and deploys.serviceId; recreates the deploySummary view; and removes serviceId from the YAML schema. The Build.services ManyToMany relation and the deploys.serviceId column are deliberately retained until then. A companion one-line lifecycle-cli PR removes the vestigial enableFullYaml type field.

Testing

  • Static gates (both commits): pnpm lint ✓, pnpm ts-check (zero new deterministic type errors vs baseline), pnpm test271 suites / 2903 tests ✓.
  • Local Tilt regression against lifecycle-examples (github/buildkit + docker + native-helm services, a requires dependency, an optional service, template variables): full build→deploy cycle reaches deployed with all services ready on the cleaned-up code. Deployables materialize purely from YAML; env-var template resolution ({{{backend_publicUrl}}}, {{{cache_internalHostname}}}) and the PATCH /api/v2/builds/{uuid}/services override path both verified. A pre-change baseline on unmodified main was captured first for comparison.
  • Remaining validation: the lifecycle-static-env remote-config-reference path (cross-repo defaultServices with explicit repository/branch) has not yet been exercised end-to-end — recommend a targeted run before merge.

Notes for reviewers

  • The fingerprint field removal changes build fingerprints once, so expect a one-time rebuild on the first deploys after rollout (cosmetic).
  • No UI or helm-chart changes are needed; the UI already consumes only the deployable payload.

@vigneshrajsb

Copy link
Copy Markdown
Contributor Author

Static-env remote-config regression: ✅ validated

The cross-repo defaultServices path (the reworked updateOrCreateDeployableUsingYamlConfig remote-fetch logic) has now been exercised end-to-end on this branch via lifecycle-static-env#2:

  • The static env's lifecycle.yaml lists service names with remote repository/branch references only. On this bridge-free code the deployables were correctly materialized by pulling lifecycle.yaml from three separate remote repos/branches (lifecycle-examples@codex/lfc-remote-config-fixture for frontend/remote-fixture-a, plus remote docker configs for backend/db).
  • Build polished-dream-384394 reached deployed with all services ready — including the remote github-source services (frontend, remote-fixture-a) built from the fixture branch.

Note: the first attempt failed building the github-source services because the local kind node was memory-overcommitted (build jobs bursting past limits); it went green on retry with staggered builds. This is a local-cluster resource limit, not a code issue — build 54 (lifecycle-examples) built its github service fine on identical code.

filterGithubRepositoryId?: number
) {
if (!build?.enableFullYaml || !reconciliationResult?.canReconcile) {
if (!reconciliationResult?.canReconcile) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Remove the deleted relation from the reconciliation query below

This path now runs without the enableFullYaml guard, but the stale-deploy lookup later in this method still eagerly fetches [build, service, deployable]. Since Deploy.service is removed in this PR, Objection rejects that query with ValidationError: unknown relation service in an eager expression, so deleting or renaming a YAML service fails before cleanup. Please change that graph to [build, deployable] and ideally cover the real relation expression in a regression test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 265f759. The stale-deploy lookup now eager-loads [build, deployable] (the removed Deploy.service relation is gone). staleDeploys is only consumed by cleanupDeploy, which never reads deploy.service, so nothing downstream needed the relation. Added a regression test in the stale deploy reconciliation suite asserting the eager expression no longer references service, so a reintroduction would fail CI.

vigneshrajsb added a commit that referenced this pull request Jul 13, 2026
…ation query

The reconcileDeletedDeployables stale-deploy lookup still eager-loaded
[build, service, deployable]. Since PR removed the Deploy.service relation,
Objection rejects that with 'unknown relation service', so deleting or
renaming a YAML service failed before cleanup could run. Change the graph
to [build, deployable] (staleDeploys is only used for cleanupDeploy, which
never reads deploy.service) and add a regression test asserting the eager
expression no longer references the removed relation.

Addresses review feedback on #228.
Lifecycle migrated fully to YAML-based per-service configuration pulled at
build time; the legacy database-config paths are unreachable (every
environment is created with enableFullYaml=true and classicModeOnly has no
writers). Remove the dead code:

- Collapse ~137 enableFullYaml / classicModeOnly conditionals across 19
  files to the YAML/deployable path (every `flag ? deploy.deployable.X :
  deploy.service.X` becomes `deploy.deployable.X`). Remove the flag writes,
  the enableFullYamlSupport getter, the queue-fingerprint field, the flag
  from API responses/swagger, and the classic-only helpers.

- Remove the DB->deployable bridge (updateOrCreateDeployableUsingDbConfig
  and friends); deployables are now materialized purely from YAML. A
  lifecycle.yaml defaultServices/optionalServices entry carrying serviceId
  now warn-and-skips instead of resolving a DB Service row.

- Delete setupDefaultBuildServiceOverrides/createBuildServiceOverride,
  ServiceService, and the BuildServiceOverride model.

- Remove the Deploy.service relation + serviceId field, Build.buildServiceOverrides,
  and Environment.services/defaultServices/optionalServices relations,
  stripping the `service` token from every graph-fetch string across the
  agent/session/build/deploy code (including the stale-deploy reconciliation
  query and the query_database AI-tool relation allowlist).

DB tables/columns are untouched; they are dropped in phase 2 after this
deploys cleanly. The Build.services ManyToMany relation and the
deploys.serviceId column are deliberately retained until then.

lint / ts-check (zero new deterministic errors) / test (271 suites,
2903+ tests) all green. Validated on local Tilt against lifecycle-examples
(github/docker/helm, dependencies, optional-service activation, override
API) and lifecycle-static-env (cross-repo remote-config references).
@vigneshrajsb vigneshrajsb force-pushed the cleanup/pr-b-remove-bridge branch from 44db80a to 8be8972 Compare July 13, 2026 22:30
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.

1 participant