fix: return error when {{ source }} references a skipped source by ambikeesshh · Pull Request #9095 · updatecli/updatecli

ambikeesshh · 2026-06-02T12:22:32Z

Summary

{{ source "id" }} returns the literal string {{ source "id" }} when the referenced source is skipped via depends_on. Since a skipped source never transitions to SUCCESS, this template literal never resolves. It silently leaks into config values causing confusing downstream failures with no clear indication of what went wrong.

What changed

Sources initialized with Result: SKIPPED, same state as an explicitly skipped source, so the template function couldn't tell them apart. Two changes fix this:

Changed initial source result from result.SKIPPED to empty string, so "not yet run" and "skipped" are distinguishable
Set source result to SKIPPED explicitly when shouldSkipResource skips a source

Then the template function can safely error on SKIPPED:

Added case result.SKIPPED in the runtime source template function that returns a clear error, consistent with existing FAILURE handling

Test

go test ./pkg/core/config/... -run TestUpdate -v -count=1
go test ./pkg/core/pipeline/... -run TestRunSources -v -count=1

New test cases:

pkg/core/config/main_test.go - unit test for SKIPPED source in template function
pkg/core/pipeline/sources_test.go - end-to-end test with a source skipped via depends_on and a target referencing it via {{ source "..." }}

ambikeesshh · 2026-06-02T12:53:31Z

hey @olblak PTAL
thanks!!

ambikeesshh · 2026-06-04T14:58:02Z

@olblak can you review it ? when you get a chance

olblak · 2026-06-04T15:48:26Z

Of course, I'll try to find some time thanks for the fix

olblak · 2026-06-04T15:49:27Z

 			Config: config.Spec.Sources[id],
 			Result: &result.Source{
-				Result: result.SKIPPED,
+				Result: "",


Why is it changed from result.SKIPPED to empty string?

Ok, now I understand.

empty string = not yet run (retry later). SKIPPED = explicitly skipped (error)
both were the same value before this change

Fix updatecli#7919

ambikeesshh · 2026-06-04T17:01:19Z

@olblak rebased on latest main

ambikeesshh · 2026-06-05T17:37:08Z

@olblak friendly ping

ambikeesshh · 2026-06-09T18:04:34Z

@olblak

olblak · 2026-06-10T12:45:34Z

@ambikeesshh Sorry very busy days at work.
Have you seen my review?

ambikeesshh · 2026-06-10T14:36:30Z

@olblak yep, i replied inline on the Result: "" change. The empty string distinguishes "not yet run" from "explicitly skipped" so the template function can error on the latter.

lemme know if you'd like any other changes

Copilot

Pull request overview

This pull request fixes a confusing failure mode where {{ source "id" }} could remain unresolved and silently leak into runtime configuration when the referenced source was skipped via depends_on. It makes “not yet run” distinguishable from “skipped”, and updates the runtime template behavior to return a clear error when a skipped source is referenced.

Changes:

Initialize sources with an empty result (instead of SKIPPED) so the runtime template engine can distinguish “pending” vs “skipped”.
Explicitly mark sources as SKIPPED when depends_on causes them to be skipped.
Update the runtime source template function to error when the referenced source is SKIPPED, and add/extend tests covering this behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
pkg/core/pipeline/sources_test.go	Adds an end-to-end test ensuring a target referencing a skipped source via `{{ source "..." }}` fails clearly.
pkg/core/pipeline/main.go	Changes initial source result to empty and explicitly sets `SKIPPED` when a source is skipped by dependency logic.
pkg/core/config/main_test.go	Adds a unit test asserting the runtime template error message for a skipped source.
pkg/core/config/funcsUpdatecli.go	Adds explicit `SKIPPED` handling in the runtime `source` template function, returning a clear error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ambikeesshh · 2026-06-14T06:43:56Z

@olblak

olblak · 2026-06-15T07:07:44Z

@olblak

Hi @ambikeesshh,
I am reposting here what I post earlier in this pullrequest

I am a bit annoyed by this change because if something unexpected happened, the resource wouldn't have a clear state.
It's used, for example, on this webapp https://app.uda.sh/updatecli/scm/dashboard to monitor Updatecli dependencies.

I think I would prefer to add a new Boolean named "IsRun" on the source/condition/target like

updatecli/pkg/core/pipeline/source/main.go

Line 22 in 094d96c

type Source struct {

Once The source function is exectued like here

updatecli/pkg/core/pipeline/source/main.go

Line 112 in 094d96c

err = source.Source(ctx, workingDir, s.Result)

then we set it to true

ambikeesshh · 2026-06-20T14:56:17Z

hey @olblak
done, reworked it the way you suggested

dropped the empty Result, sources now init to SKIPPED like conditions/targets so the state stays valid
added IsRun to source/condition/target, set to true inside each Run() where you pointed

one note: a skipped source never hits Run(), so i set IsRun = true on the skip path too. so it means "ran or skipped". if you'd rather it strictly mean "executed", i'll use a separate flag instead. let me know

thanks for the review!

olblak · 2026-06-26T06:50:26Z

thanks for the update, I'll review this pr in the coming days

olblak · 2026-06-26T07:50:28Z

sorry for the confusion on this pullrquest, I now realize that all the comments that I made on this pr where only visible to me because they were "pending"

cfr https://github.com/orgs/community/discussions/10369

ambikeesshh · 2026-06-26T07:55:43Z

@olblak
haha, no worries. thanks for the review tho :)

olblak

I manually tested using the following manifest

name: Test sourceid {{ source "shell" }}

sources:
  shell:
    name: Show 1.2.3
    kind: shell
    dependson:
      - condition#default
    spec:
      command: 'echo "1.2.3"'

  shell2:
    name: Show source output {{ source "shell" }}
    kind: shell
    spec:
      command: 'echo {{ source "shell" }}'

conditions:
  default:
    name: "False"
    kind: shell
    disablesourceinput: true
    spec:
      command: |
        false

And it works as expected.
While testing it, I spotted a minor issue which I'll fix in a follow up pr

ambikeesshh · 2026-06-26T13:19:51Z

@olblak
the e2e (Run End to End tests) is red on the latest run, and it's from this change.

turns out a bunch of the repo's own deps: bump manifests hit the new behavior: a source gets skipped via dependson (e.g. actions/checkout when release v4 isn't found → source#branch skipped), but the targets still reference {{ source "branch" }}. on main that rendered as a literal silently, now it errors with "parent source ... was skipped" so ~45 pipelines fail and the e2e assertion trips.

is that covered by the follow-up you mentioned, or would you rather skipped sources also skip their dependents instead of erroring? happy to go either way, just lmk which you prefer

olblak · 2026-06-26T16:14:34Z

This is not related to my follow up.
Mine is about initializing report data.

I think it would make more sense to not error on skipped source but just to skip dependents resources

ambikeesshh force-pushed the fix/source-template-skipped branch from 5fa3757 to 06f0a18 Compare June 2, 2026 12:30

olblak reviewed Jun 4, 2026

View reviewed changes

fix: return error when {{ source }} references a skipped source

8bf3606

Fix updatecli#7919

ambikeesshh force-pushed the fix/source-template-skipped branch from dfca649 to 8bf3606 Compare June 4, 2026 16:53

Merge branch 'main' into fix/source-template-skipped

093e8fe

olblak requested a review from Copilot June 10, 2026 17:29

Copilot started reviewing on behalf of olblak June 10, 2026 17:29 View session

Copilot AI reviewed Jun 10, 2026

View reviewed changes

ambikeesshh requested a review from olblak June 10, 2026 18:04

olblak and others added 3 commits June 15, 2026 09:07

Merge branch 'main' into fix/source-template-skipped

53b1268

fix: track resource run state with IsRun for skipped sources

89f4acb

Merge branch 'main' into fix/source-template-skipped

c1ceaed

Merge branch 'main' into fix/source-template-skipped

74a6e40

olblak added bug Something isn't working core All things related to Updatecli core engine labels Jun 26, 2026

olblak approved these changes Jun 26, 2026

View reviewed changes

olblak added 3 commits June 30, 2026 10:05

Merge branch 'main' into fix/source-template-skipped

4f88ca5

Merge branch 'main' into fix/source-template-skipped

df529f8

Merge branch 'main' into fix/source-template-skipped

7c824c0

Uh oh!

Uh oh!

Conversation

ambikeesshh commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Test

Uh oh!

ambikeesshh commented Jun 2, 2026

Uh oh!

ambikeesshh commented Jun 4, 2026

Uh oh!

olblak commented Jun 4, 2026

Uh oh!

olblak Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

olblak Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

ambikeesshh Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

ambikeesshh commented Jun 4, 2026

Uh oh!

ambikeesshh commented Jun 5, 2026

Uh oh!

ambikeesshh commented Jun 9, 2026

Uh oh!

olblak commented Jun 10, 2026

Uh oh!

ambikeesshh commented Jun 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ambikeesshh commented Jun 14, 2026

Uh oh!

olblak commented Jun 15, 2026

Uh oh!

ambikeesshh commented Jun 20, 2026

Uh oh!

olblak commented Jun 26, 2026

Uh oh!

olblak commented Jun 26, 2026

Uh oh!

ambikeesshh commented Jun 26, 2026

Uh oh!

olblak left a comment

Choose a reason for hiding this comment

Uh oh!

ambikeesshh commented Jun 26, 2026

Uh oh!

olblak commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ambikeesshh commented Jun 2, 2026 •

edited

Loading