fix: infer the repository of docker and aurora-restore services from their config by vigneshrajsb · Pull Request #250 · GoodRxOSS/lifecycle

vigneshrajsb · 2026-09-29T20:40:08Z

Description

Docker and aurora-restore services have no repository field, so their deployables were saved with no repository and their deploys with repo 0. Pushes to the repo that defines them never redeployed them, and redeploying one of them redeployed every such service in the environment.
They now take the repository and branch of the lifecycle.yaml that defines them, the same identity a helm or github service gets when it points at its own config repo. defaultBranchName stays unset. Scoped runs, deploy rows and single-service redeploys then work through the existing repositoryId paths.
An existing deploy row is corrected the next time a run finds it doesn't match its deployable's repository, so no migration is needed.
Push handling doesn't let these rows stand in for real services, so a config repo that only defines them still gets the static-environment fallback.
Stale reconciliation keeps a dependency that another service in the build still requires.
PR comment edits no longer save a Docker service's image@tag display label as its branch override. That label pinned the service outside its config's branch, which kept it out of branch-scoped runs in any env whose comment had been edited. The active/inactive toggle still applies.

Verifying Changes

Unit tests cover identity inference, excluded types (chart-only helm, externalHTTP), deploy scoping and row correction, the static-env push fallback, the shared-dependency guard, the PR comment link, and comment edits on Docker services. Lint and tests pass, and typecheck has no new errors vs main.
End to end on a local kind cluster, each scenario compared with a baseline on main:
- PR and static environments with mixed branches.
- A config-only static env.
- Parent disabled with the dependency still active.
- A shared dependency dropped by one parent.
- Rows reset to the main shape, healing on the first run.
- PR comment links, and overrides still rejected for Docker services.
- A human comment edit leaving Docker branch overrides empty while still toggling a Docker service off.

Notes

Behavior changes:
- Docker dependencies restart when their parent is pushed, the same as helm and github services. Persistence still comes from serviceDisks.
- {{<dep>_branchName}} resolves to the config's branch.
- PR comments link these services to their config repository.
Existing Docker rows that already carry an image@tag override need a one-time cleanup after this ships (commentBranchName = NULL on docker deployables).
With features.reconcileDeletedServices on, dependencies already removed from config are cleaned up on their next scoped run.
Out of scope: helm services that reference only a chart, and externalHTTP services, still have no repository.

Docker and aurora-restore services have no repository of their own, so their deployables were saved with no identity and their deploys with repository 0. Repo-scoped runs never matched them, and redeploying one service picked up every repo-less deploy in the environment. They now take the identity (resolvedFromRepositoryId) and tracked branch of the config they were read from, and existing deploy rows are corrected the next time that config resolves. Push handling ignores these rows when looking for real services, so the static-environment fallback still runs. Stale reconciliation keeps a dependency that another service in the build still requires.

vigneshrajsb

Inline walkthrough of each change, for reviewers.

vigneshrajsb · 2026-09-29T20:45:39Z

+      const deployType = YamlService.getDeployType(service);
+      const isRepoLess =
+        repoName == null && (deployType === DeployTypes.DOCKER || deployType === DeployTypes.AURORA_RESTORE);
+      if (isRepoLess) branch = branchName;


Where the repo ID comes from. repositoryId here is the repo whose lifecycle.yaml defines this service, not the repo of the service that requires it:

A service defined locally resolves against rootRepository and the root YAML (~L531).

A service defined in another repo is resolved from that repo's fetched YAML and gets passed repository.githubRepositoryId (~L606/L621). Its requires are resolved from the same file, so they get the same repo.

isRepoLess is true only when the service has no repository of its own and is docker or aurora-restore. Chart-only helm and externalHTTP services are left alone (out of scope).

For those services, branch is set to the branch that YAML was read from, so they follow that config's pushes. defaultBranchName is not set, so they never get picked as default-branch-tracked services on their own.

vigneshrajsb · 2026-09-29T20:45:40Z

        parentDeployableName,
-        build
+        build,
+        isRepoLess ? repositoryId ?? null : null


This passes the defining config's repo ID down, but only for repo-less services. Every other type passes null, so their attributes stay the same.

vigneshrajsb · 2026-09-29T20:45:40Z

-          resolvedFromRepositoryId: repositoryId != null ? Number(repositoryId) : null,
+          resolvedFromRepositoryId:
+            repositoryId != null
+              ? Number(repositoryId)


resolvedFromRepositoryId is set to the defining config's repo when the service has no repo of its own. Before this change it was null for docker and aurora-restore services.

repositoryId stays null: these services have no code repo, and callers that need an actual code repo (clone, image build, commit links) still see none.

vigneshrajsb · 2026-09-29T20:45:40Z

        const uuid = `${deployable.name}-${build?.uuid}`;
        const patchFields: Objection.PartialModelObject<Deploy> = {};
        const deployableRepositoryId = Number(deployable.repositoryId);
+        const scopeRepositoryId =


Scope repo. This is the repo a deploy row belongs to for repo-scoped runs:

Services with their own repo keep deployable.repositoryId, so they're unchanged.

Repo-less services use resolvedFromRepositoryId, the repo whose config defines them (see deployable.ts).

Before this change, Number(null) made this 0, so these services never matched a scoped run and all shared one 0 bucket.

vigneshrajsb · 2026-09-29T20:45:40Z

        const isTargetSource =
          !githubRepositoryId ||
-          (deployableRepositoryId === githubRepositoryId && (!sourceBranch || effectiveBranch === sourceBranch));
+          (scopeRepositoryId === githubRepositoryId && (!sourceBranch || effectiveBranch === sourceBranch));


isTargetSource now compares against the scope repo. A push to the defining repo and branch (or a scoped redeploy) now includes its docker/aurora services with their parent, instead of skipping them.

vigneshrajsb · 2026-09-29T20:45:40Z

          patchFields.uuid = uuid;
          patchFields.branchName = effectiveBranch;
          patchFields.tag = deployable.defaultTag;
+          if (deployable.repositoryId == null && Number(deploy.githubRepositoryId) !== scopeRepositoryId) {


This corrects rows that already exist: a repo-less deploy still at githubRepositoryId = 0 is fixed the first time a run targets it.

It's why there's no migration. Rows created before this change, including static envs, fix themselves on their first scoped or full run. Until then they behave exactly as they do today.

This runs only when isTargetSource is true, so an unrelated run never rewrites the row.

vigneshrajsb · 2026-09-29T20:45:40Z

            uuid,
            internalHostname: uuid,
-            githubRepositoryId: deployableRepositoryId,
+            githubRepositoryId: scopeRepositoryId,


New rows are created with the scope repo instead of Number(null) = 0. Single-service redeploy looks up deploys by this column, so a docker service is now redeployed together with its own repo's services, not with every repo-less service in the env.

vigneshrajsb · 2026-09-29T20:45:40Z

+          .where('active', true)
+          .whereNot('status', 'torn_down')
+          .withGraphFetched('[build.[pullRequest], deployable]')
+      ).filter(


This keeps the static-env fallback working. Repo-less rows now match githubRepositoryId + branchName for their defining config's repo.

Without this filter, a push to a config repo that defines only docker/aurora services (for example a static env's own config repo) would make allDeploys non-empty. The handlePushForStaticEnv fallback below would then be skipped. The rows would also be dropped anyway by deploysToRebuild, since they have no defaultBranchName, so the push would do nothing.

These rows are still redeployed: a real service's row selects the build, and the scoped run then includes them through findOrCreateDeploys. This was reproduced locally before the fix, and the new tests in githubAutoTrack.test.ts cover it.

vigneshrajsb · 2026-09-29T20:45:40Z

+    if (staleCandidates.length > 0) {
+      await build.$fetchGraph('deployables');
+      const candidateNames = new Set(staleCandidates.map((deployable) => deployable.name));
+      const stillRequired = new Set(


This guards shared dependencies. Now that repo-less services have an owner, they can be reaped by stale reconciliation (when reconcileDeletedServices is on).

A dependency required by two parents is one row per build, and its owner is whichever scope resolved it last. Without this guard, parent A dropping it from requires would reap it while parent B in the same build still requires it.

The fix skips any stale candidate that another deployable in the build still lists in requires. The only failure mode is keeping a row, never removing one wrongly. This was reproduced on a mixed-branch static env, and the new test in build.test.ts covers it.

…their config These services have no repository field, so give their deployables the repository and branch of the config that defines them, the same identity a helm or github service gets when it points at its own config repo. Deploy scoping then needs no special case, and existing deploy rows are corrected whenever they no longer match their deployable's repository. PR comments now link these services to their config repository.

vigneshrajsb

Updated walkthrough after the rework in 6796fef. Docker and aurora-restore deployables now get their repositoryId from the repo whose config defines them. The earlier comments are outdated, except the build.ts one on the shared-dependency guard, which still applies.

vigneshrajsb · 2026-09-29T21:28:54Z

+      // Docker and aurora-restore services have no repository field; they belong to the repo whose config defines
+      // them, the same as a helm or github service whose repository is that config's own repo.
+      const deployType = YamlService.getDeployType(service);
+      const inheritsConfigRepository =


Where identity is assigned (the only type check for identity). getRepositoryName reads github/codefresh/helm.repository; docker and auroraRestore blocks have no such field, so on main these deployables got repositoryId = null. Everything downstream is copied from that: resolvedFromRepositoryId, deploy.githubRepositoryId = Number(null) = 0, and the scope of repo-scoped runs.

repositoryId (this function's argument) is the repo whose lifecycle.yaml is being parsed:

a service in the env's own config resolves against rootRepository with the root YAML;

a service from another repo is resolved from that repo's fetched YAML (repository.githubRepositoryId is passed at the two call sites below), and its requires come from the same file.

Chart-only helm and externalHTTP services are left as they are (null); that's a separate follow-up.

vigneshrajsb · 2026-09-29T21:28:54Z

        buildUUID,
-        repository?.githubRepositoryId ?? null,
-        branch,
+        inheritsConfigRepository ? repositoryId ?? null : repository?.githubRepositoryId ?? null,


For these types, the deployable gets the config repo as repositoryId and the branch that config was read from. That's the same thing the branch ternary above produces for a helm or github service whose repository is the config's own repo, so these services behave like a helm service pointing at its own config repo.

generateAttributesFromYamlConfig is unchanged: resolvedFromRepositoryId follows repositoryId, and defaultBranchName stays unset because getBranchName returns nothing for these types. Clone, build and SHA paths all check the service type first, so Docker services still aren't cloned or built and get no SHA lookup. The visible change is that PR comments now link them to config repo/tree/branch.

vigneshrajsb · 2026-09-29T21:28:54Z

          patchFields.branchName = effectiveBranch;
          patchFields.tag = deployable.defaultTag;
+          if (deployable.repositoryId != null && Number(deploy.githubRepositoryId) !== deployableRepositoryId) {
+            patchFields.githubRepositoryId = deployableRepositoryId;


This corrects existing rows, and it isn't type-specific: when a targeted run finds a deploy whose githubRepositoryId no longer matches its deployable's repositoryId, it patches the row. For rows that already match (every github/helm row) this does nothing.

It's what makes a migration unnecessary. A Docker row created on main (repo 0) is fixed on the first run that resolves its config, because the deployable is re-resolved earlier in that same run. Until then it behaves exactly as it does on main.

The rest of this function needed no change. Scoping, isTargetSource and row creation already use Number(deployable.repositoryId), which is now correct for these types.

vigneshrajsb · 2026-09-29T21:28:54Z

+          .withGraphFetched('[build.[pullRequest], deployable]')
+      ).filter(
+        (deploy) =>
+          deploy.deployable?.type !== DeployTypes.DOCKER && deploy.deployable?.type !== DeployTypes.AURORA_RESTORE


Why a type filter is still needed here. These rows now match the push's repo and branch, but they have no defaultBranchName, so deploysToRebuild drops them anyway. They can't select a build. What they could do is make allDeploys non-empty for a config repo that defines only Docker/Aurora services (for example a static env's own config repo). That would skip the handlePushForStaticEnv fallback, and the push would do nothing. I reproduced this locally before adding the filter.

This matches main for these types: their rows were at repo 0 and never matched a push. They still redeploy along with their parent, because the parent's row selects the build and the scoped run includes them. Both cases are covered in githubAutoTrack.test.ts.

A PR comment lists docker services with their image@tag as a display value. Any comment edit re-applied every row, saving that label as the service's branch override, which pinned it outside its config's branch and dropped it from branch-scoped runs. Comment edits now keep only the active state for docker services; the image still comes from the config.

vigneshrajsb

Walkthrough for 5cb6cd8.

vigneshrajsb · 2026-09-30T16:15:13Z

    const deployable = deploy.deployable!;
+    // A Docker row's value in the comment is its image@tag display label, not a branch. Saving it as a branch
+    // override would pin the service to that label and drop it from its config's branch-scoped runs.
+    if (deployable.type === DeployTypes.DOCKER) branchOrExternalUrl = undefined;


Why this is here. Editing a PR comment runs applyBuildOverrides, which re-applies every service row it parses from the comment, not just the rows the person changed. For Docker services the row value is the image@tag display label from getDockerDisplayValue, so any human edit, even ticking "Redeploy", saved that label as commentBranchName.

The API path (applyServiceOverrides) already drops these through validateServiceOverrides; the comment path never ran that check. The label never changed the image, which always comes from dockerImage + defaultTag. What it did change was the effective branch (commentBranchName ?? branchName), so the service stopped matching its config's branch and was skipped by branch-scoped runs.

Only Docker is guarded. Codefresh and configuration rows carry real branches that people do change through the comment, and aurora-restore services have no row in the comment. Toggling a Docker service on or off still applies. Covered by the new test in override.test.ts, and checked locally with a real comment edit.

vigneshrajsb added 2 commits September 29, 2026 12:25

test: cover a push matching both real and repo-less services

5dc9694

vigneshrajsb commented Sep 29, 2026

View reviewed changes

vigneshrajsb changed the title ~~fix: give repo-less dependencies their parent config's ownership~~ fix: key repo-less dependencies to the repo whose config defines them Sep 29, 2026

vigneshrajsb commented Sep 29, 2026

View reviewed changes

vigneshrajsb changed the title ~~fix: key repo-less dependencies to the repo whose config defines them~~ fix: infer the repository of docker and aurora-restore services from their config Sep 29, 2026

vigneshrajsb marked this pull request as ready for review September 29, 2026 21:49

vigneshrajsb requested a review from a team as a code owner September 29, 2026 21:49

vigneshrajsb commented Sep 30, 2026

View reviewed changes

vigneshrajsb merged commit 479a4a0 into main Sep 30, 2026
5 checks passed

Conversation

vigneshrajsb commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Verifying Changes

Notes

Uh oh!

vigneshrajsb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vigneshrajsb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vigneshrajsb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vigneshrajsb commented Sep 29, 2026 •

edited

Loading