SRVOCF-1038: Show build status and pipeline failures in the functions list

[matejvasek]

Summary

• 🎁 Surface GitHub Actions build status (Building / BuildFailed, with a link to the run) in the functions list
• 🎁 Live updates over SSE (/api/v1/func/build/watch), user-scoped like /list
• 🔧 scm.Client.WatchWorkflowRuns polls GitHub Actions behind one channel per connection, scoped to func-deploy.yaml, ETag-cached to keep 304s free against the rate limit
• 🧪 Ginkgo tests (handler, scm/github, fakegithub); Vitest tests (useBuildStatus, list merge); Playwright e2e against the real backend and fakegithub
• 📚 Design spec in docs/design/2026-08-26-SRVOCF-1038-build-status-design.md, implementation plan in docs/plans/completed/2026-08-26-SRVOCF-1038-build-status.md

Fixes SRVOCF-1038

How the build status stream works

The backend polls GitHub for workflow runs and the browser subscribes to the result. Server-Sent Events (SSE) is a long-lived HTTP response of Content-Type: text/event-stream that the server keeps open and appends text frames to. It is one-way (server to client), which is all we need here.

browser                       console proxy            backend                    GitHub
   |                                |                     |                          |
   | GET .../func/build/watch       |                     |                          |
   | X-SCM-Token: <PAT>             |                     |                          |
   |------------------------------->|-------------------->|                          |
   |                                |                     | ListRepos (synchronous)  |
   |                                |                     |------------------------->|
   |                                |                     |   401 -> plain HTTP 401  |
   |<-- 200 text/event-stream ------|<--------------------|                          |
   |                                |                     |  every 3s: latest run    |
   |                                |                     |  per repo (ETag -> 304)  |
   |                                |                     |------------------------->|
   |<-- event: build-status --------|<--------------------|  (only when changed)     |
   |<-- ":" heartbeat every 15s ----|<--------------------|                          |

Why polling and not webhooks. GitHub can push workflow_run events to a webhook, and that would be lower latency, but it is a much bigger system: a publicly reachable route into the cluster, a webhook plus signing secret registered and kept in sync on every function repo, signature verification, and server-side state to fan each event out to the right browser session. Polling needs none of that. It runs inside the existing user-scoped request, holds no state beyond the life of the connection, and unchanged polls are 304s, so the steady-state cost is close to zero. The push we do need, backend to browser, is the one SSE gives us.

Wire format. Frames are separated by a blank line. A line starting with : is a comment (our heartbeat, which keeps proxies from closing an idle connection). Everything else the client ignores unless the frame's event: is build-status:

event: build-status
data: {"functions":{"alice/hello":{"buildStatus":"Building","runURL":"https://github.com/..."}}}

:

event: build-status
data: {"functions":{"alice/hello":{"buildStatus":"Failed","runURL":"https://github.com/..."}}}

Each frame is a full snapshot, not a delta: the map is keyed by owner/repo and the client replaces its state wholesale. That makes the client stateless with respect to ordering and missed frames, and it means a reconnect needs no catch-up protocol.

Auth failures stay ordinary HTTP. Once a response is a stream you can no longer change its status code, so HandleBuildWatch does repo discovery before writing any headers. A revoked PAT is therefore a plain 401, not a half-written stream (backend/handler/build.go:50-59).

Change-only emission. The poller compares each new snapshot to the previous one and only sends on the channel when it differs, so an idle list produces nothing but heartbeats (backend/scm/github/watch.go:42-53). Polls that find nothing new are 304 Not Modified thanks to an ETag-caching transport, and 304s do not count against GitHub's primary rate limit.

Why not EventSource? The browser's built-in SSE client cannot set request headers, which would force the PAT into the URL (where it lands in logs and history). So the client uses consoleFetch with timeout: 0 and reads response.body as a ReadableStream, splitting frames on \n\n itself (src/common/clients/useBuildStatus.ts). The cost is that we implement reconnect by hand: 3s backoff on a dropped stream, and a hard stop on 401/403 since a bad token will not fix itself. Full rationale in the design doc under "Transport decision: SSE over consoleFetch stream".

Where it lands in the UI. useBuildStatus returns a map that FunctionsListPage merges with the cluster status per function. The merge is non-destructive: a function the cluster knows about keeps its cluster badge and the build shows only as a secondary indicator (spinner or red warning icon linking to the run).

Suggested review order

The change is easier to follow outside-in rather than by diff order:

1. docs/design/2026-08-26-SRVOCF-1038-build-status-design.md for the what and why, especially the status merge table.
2. backend/scm/github/watch.go for the polling loop, change detection, and per-repo error carry-forward.
3. backend/handler/build.go for the SSE framing and the auth-before-headers ordering.
4. src/common/clients/useBuildStatus.ts for the client-side frame parsing and reconnect.
5. src/pages/function-list/FunctionsListPage.tsx (mergeBuild) and components/FunctionTable.tsx (StatusCell) for how the two statuses combine and render.
6. Tests last; e2e/use-cases/list/build-status.test.ts is the end-to-end story in one file.

SSE resources

• MDN: Using server-sent events (start here; the "Event stream format" section is the whole wire protocol)
• WHATWG HTML spec: Server-sent events (normative, for the edge cases)
• MDN: EventSource (the API we deliberately do not use, see above)
• MDN: Using readable streams (what response.body gives us instead)
• Go: http.Flusher (why every frame is followed by a Flush())
• nginx: proxy_buffering / X-Accel-Buffering (why the handler sets that header)

Checklist

• Updated docs/ARCHITECTURE.md (if there are relevant changes to our layered architecture)

Additional Info

• Build status is non-destructive over a function the cluster knows about: it keeps its cluster badge with a secondary build indicator, so the cluster state is never misrepresented. The gate is cluster presence rather than the status value, because a live function reports Deploying for a moment mid-rollout and gating on Running/ScaledToZero made every redeploy flicker through Building.
• No failure reason is surfaced in the list. The run is one click away from the badge, and reproducing GitHub's job output in a table cell was not worth the extra API calls.

🤖 Generated with Claude Code

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[matejvasek]

/test all

[matejvasek]

/test e2e-aws

[openshift-ci-robot]

@matejvasek: This pull request references SRVOCF-1038 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Summary

• 🎁 Surface GitHub Actions build status (Building / BuildFailed, with failure reason and a link to the run) in the functions list
• 🎁 Live updates over SSE (/api/v1/func/build/watch) plus a snapshot endpoint (/api/v1/func/build/status), user-scoped like /list
• 🔧 scm.Client.LatestWorkflowRun via GitHub Actions, scoped to func-deploy.yaml, ETag-cached to keep 304s free against the rate limit
• 🧪 Ginkgo tests (handler, scm/github, fakegithub); Vitest tests (useBuildStatus, list merge); Playwright e2e against the real backend and fakegithub
• 📚 Design spec in docs/design/2026-08-26-SRVOCF-1038-build-status-design.md

Fixes SRVOCF-1038

Checklist

• Updated docs/ARCHITECTURE.md (if there are relevant changes to our layered architecture)

Additional Info

• Build status is non-destructive over an available function: Running/ScaledToZero keep their cluster badge with a secondary build indicator, so availability is never misrepresented.
• Deferred: non-destructive treatment for a cluster Error (needs gating on cluster presence, not the status string, since Error is overloaded with a repo/list-level error). Noted in the design doc.
• Draft while the xhigh self-review findings are triaged (no high-severity; remainder are low / prototype-scope).

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[matejvasek]

/test e2e-aws

[twoGiants]

Next backend review batch.

[twoGiants]

Inline readSSEData here. No value in extracting it, readSSEDataWithin is already a small function.

[twoGiants]

I see a few things here and it extends to watch.go:

1. buildStatusItem duplicates scm.WorkflowRun. The only addition is BuildStatus (the derived string). That derivation belongs in the scm layer, not the handler, because it's business logic. The handler is the (REST) API layer and should only take care of managing requests and responses: get what it needs from the request, forward it downstream, and prepare data to be sent back in the proper format (SSE here).

2. deriveBuildStatus is domain logic, not HTTP handling. Mapping GitHub Actions statuses (queued, in_progress, completed/failure) to build statuses (Building, Succeeded, Failed, None) is business logic. The handler should just serialize what the scm layer gives it.

3. toSnapshot transforms []scm.RepoRun into a handler-local type. The handler should receive data ready to serialize.

Proposed refactoring: derive the build status in watch.go where the run is already constructed from run.GetStatus(). Add json tags to scm.RepoRun and scm.WorkflowRun so they serialize with proper field names. Then send []scm.RepoRun directly as the SSE data payload. The handler only does json.Marshal(runs) and puts it into the event: build-status\ndata: ...\n\n frame. buildStatusItem, buildSnapshot, toBuildStatusItem, deriveBuildStatus, and toSnapshot can go away.

The frontend adapts by converting the array to a keyed lookup in the hook (one conversion, one place).

Thoughts?

[matejvasek]

Sounds reasonable.

[twoGiants]

Why not send an SSE error event before closing when there is a stream error? You could then also better test for it.

[matejvasek]

I was thinking about it. I even experimented with that locally. But chosen to defer.

There was even comment:

// End the stream so the client reconnects and takes its re-auth path.
// We could also potentially push error event to the stream.

We could also potentially push error event to the stream.

But you only told me to delete the comment 😄 .

[twoGiants]

As a reminder from our sync. We agreed on sending an error event.

[twoGiants]

Spying on the Stop() call seems not necessary if you derive the pollCtx in watch.go from the callers ctx. Then when the request context is cancelled (client disconnects), the poll goroutine stops automatically.

You made this design choice deliberately. Can you tell me why, I am curious?

It looks like in our case it would be fine to have the watch tied to the request lifetime.

[twoGiants]

This works without the spy. Same test:

	It("stops the watch when the client disconnects", func() {
		ch := make(chan scm.WorkflowRunsOrErr)
		stub := &scm.ClientStub{
			OnWatchWorkflowRuns: func(ctx context.Context, workflowFile string) (scm.WorkflowWatch, error) {
				return &testWatch{ch: ch}, nil
			},
		}

		mux := http.NewServeMux()
		mux.HandleFunc("GET /watch", buildWatchWithStub(stub, handler.WithHeartbeat(fastHeartbeat)))
		ts := httptest.NewServer(mux)
		DeferCleanup(ts.Close)

		ctx, cancel := context.WithCancel(context.Background())
		req, err := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL+"/watch", nil)
		Expect(err).NotTo(HaveOccurred())
		req.Header.Set("X-SCM-Token", "pat")

		resp, err := ts.Client().Do(req)
		Expect(err).NotTo(HaveOccurred())
		DeferCleanup(func() { resp.Body.Close() })

		reader := bufio.NewReader(resp.Body)
		_, err = reader.ReadString('\n')
		Expect(err).NotTo(HaveOccurred())

		cancel()

		select {
		case _, ok := <-ch:
			Expect(ok).To(BeFalse(), "expected channel to close when request is cancelled")
		case <-time.After(2 * time.Second):
			Fail("expected watch channel to close when request is cancelled")
		}
	})

[twoGiants]

Another small backend batch review.

[twoGiants]

nit

Suggested change

	func (c *ghClient) WatchWorkflowRuns(ctx context.Context, workflowFile string) (scm.WorkflowWatch, error) {
	func (c *ghClient) WatchWorkflowRuns(ctx context.Context, workflowFilename string) (scm.WorkflowWatch, error) {

[twoGiants]

As a reminder from our sync. We agreed on sending an error event.

[twoGiants]

This works without the spy. Same test:

	It("stops the watch when the client disconnects", func() {
		ch := make(chan scm.WorkflowRunsOrErr)
		stub := &scm.ClientStub{
			OnWatchWorkflowRuns: func(ctx context.Context, workflowFile string) (scm.WorkflowWatch, error) {
				return &testWatch{ch: ch}, nil
			},
		}

		mux := http.NewServeMux()
		mux.HandleFunc("GET /watch", buildWatchWithStub(stub, handler.WithHeartbeat(fastHeartbeat)))
		ts := httptest.NewServer(mux)
		DeferCleanup(ts.Close)

		ctx, cancel := context.WithCancel(context.Background())
		req, err := http.NewRequestWithContext(ctx, http.MethodGet, ts.URL+"/watch", nil)
		Expect(err).NotTo(HaveOccurred())
		req.Header.Set("X-SCM-Token", "pat")

		resp, err := ts.Client().Do(req)
		Expect(err).NotTo(HaveOccurred())
		DeferCleanup(func() { resp.Body.Close() })

		reader := bufio.NewReader(resp.Body)
		_, err = reader.ReadString('\n')
		Expect(err).NotTo(HaveOccurred())

		cancel()

		select {
		case _, ok := <-ch:
			Expect(ok).To(BeFalse(), "expected channel to close when request is cancelled")
		case <-time.After(2 * time.Second):
			Fail("expected watch channel to close when request is cancelled")
		}
	})

[twoGiants]

Next review batch for the backend.

Going down deeper I noticed that the WatchWorflowRun resides in the wrong place, the SCM, which is about code management -> watching a CI pipeline run to completion is a different concern.

See my comments and proposals below.

[twoGiants]

After migrating the logic for this test to the watcher, don't forget to move the test too.

[twoGiants]

Move the test double next to the interface and rename to WorkflowWatchFake. There are now a few of those, please reduce them to a minimum. We probably need one WorkflowWatchFake and one WorkflowWatchStub and that's it.

[twoGiants]

Can you inline the buildStatusFor function here? Will be a bit easier to read this code.

[twoGiants]

You can also remove the comments here in the test which were meant for me and which mention "frontend". Our API is consumed by a frontend client now, but it could be any client.

[twoGiants]

1. Conclusion: is defined but not used anywhere except when you derive the status later in the handler. We agreed on deriving the status when you collect the data you need for it in the watch.go -> it means you don't need to store the Conclusion. It's also unused in the UI. Drop it.

2. Status: as aligned in sync, should only store our final status. We don't need to mirror and store GitHubs status only to derive our own later.

3. ID: unused in BE and UI. Why keep it? For a later scenario? For which? In any case you can add it back later once needed. I'd drop it.

4. HeadSHA: unused in BE and UI. I'd do the same as with ID.

5. HTMLURL: rename to just URL or RunURL, but I think WorkflowRun.URL reads better then WorkflowRun.RunURL.

I'd word the comment decoupled from Github and the WorkflowRun or more general CIRun should probably look like that:

// CIRun is the latest CI run for a function's repository. A nil
// *CIRun means the repository has no runs on its default branch.
type CIRun struct {
	Status string // Building | Succeeded | Failed | None
	URL    string // link to the run in the CI provider
}

[twoGiants]

remove comment

[twoGiants]

WatchWorkflowRuns doesn't belong on the SCM client interface. SCM is about code management (repos, files, commits). Watching a CI pipeline run to completion is a different concern.

This should live in its own package (e.g. ci or pipeline) with its own client interface. The current GitHub Actions implementation would be one provider, with Tekton coming next and potentially others later. Keeping it separate now avoids having to pull it out when we add Tekton support.

Strictly speaking, StoreSecret, StoreVariable, and GetVariable are also CI concerns living on the SCM interface. Let's create a ticket to pull those out too in a follow-up PR, no need to do now.

[matejvasek]

Yes, I myself was not sure whether I should create new package. Give the SCM already contained the mentioned variable/secret API I assumed that SCM == complete forge with all batteries included. But given that we should add Tekton latter it would make sense to have it separate.

[twoGiants]

Can you move this stub implementation below the WorkflowWatch interface and rename it to WorkflowWatchStub?

[twoGiants]

Next batch of backend review. I'm done with scm/github/client.go, next will be watch.go.

[twoGiants]

remove comment

[twoGiants]

When you move the watch implementation to a new package you can move out all the watch specific configuration there too.

Lets align on a structure of such files, wdyt about:

1. The core struct(s) at the top
2. The constructors / factory functions next
3. Builder pattern types and functions next
4. The core struct(s) methods and functions next
5. Supporting struct(s) next
6. Supporting struct(s) methods and functions next

Lets put what we agree on into the STYLEGUIDE.md.

[twoGiants]

Wdyt? -> Extracting forceRevalidate, drainOnClose, and the cache setup into backend/scm/github/httpcache.go with a newHTTPCache() constructor. NewWithBaseURL calls it and gets back a ready-to-use http.RoundTripper.

Keeps client.go focused on the SCM operations.

[twoGiants]

It took me a while to understand the http cache role and the comments and the forceRevalidate and drainOnClose workaround. Claude was of great help.

What I got now is the following.

First about httpcache. It does quite a few things and two of them are:

1. Freshness caching: serve from cache without contacting the server (the max-age window)
2. Validation caching: store ETags, send If-None-Match, replay cached body on 304
3. and "many" more things.

We disable 1. entirely with forceRevalidate. We only use 2., we don't use any other features.

We're pulling in an archived dependency with a known bug, working around that bug with drainOnClose, and then disabling part of its functionality with forceRevalidate and don't use the rest. All to get ETag storage and 304 replay.

I see a case for writing this small peace of code ourself. Here is a draft (tested):

type cacheEntry struct {
	etag string
	body []byte
}

type etagCache struct {
	next    http.RoundTripper
	mu      sync.Mutex
	entries map[string]*cacheEntry
}

func newETagCache(next http.RoundTripper) *etagCache {
	return &etagCache{next: next, entries: make(map[string]*cacheEntry)}
}

func (c *etagCache) RoundTrip(req *http.Request) (*http.Response, error) {
	if req.Method != http.MethodGet {
		return c.next.RoundTrip(req)
	}

	key := req.URL.String()

	c.mu.Lock()
	entry := c.entries[key]
	c.mu.Unlock()

	req = req.Clone(req.Context())
	if entry != nil {
		req.Header.Set("If-None-Match", entry.etag)
	}

	resp, err := c.next.RoundTrip(req)
	if err != nil {
		return nil, err
	}

	if resp.StatusCode == http.StatusNotModified && entry != nil {
		resp.Body.Close()
		resp.Body = io.NopCloser(bytes.NewReader(entry.body))
		resp.StatusCode = http.StatusOK
		return resp, nil
	}

	etag := resp.Header.Get("ETag")
	if etag != "" && resp.Body != nil {
		body, readErr := io.ReadAll(resp.Body)
		resp.Body.Close()
		if readErr != nil {
			return nil, readErr
		}
		c.mu.Lock()
		c.entries[key] = &cacheEntry{etag: etag, body: body}
		c.mu.Unlock()
		resp.Body = io.NopCloser(bytes.NewReader(body))
	}

	return resp, nil
}

In any case, I'd extract that code into a etagcache.go if we use my proposal. And if you want to keep the existing code then into httpcache.go as proposed the comment above.

Wdyt?

[matejvasek]

TBH I would not use httpcache library if it was not already our indirect dependency, I would implement it by myself.

[twoGiants]

So, what do you want to do now?

[matejvasek]

Given the two issues (max-age,body-not-read-to-end) I would probably chosen to implement it ourselves.

[pmeida]

Required change since #185 came first 😕.

[pmeida]

#185 has merged and with it we started only listing repos that were created by the present cluster.

This returns all GitHub repos without applying the cluster URL filter that the list handler uses. The watcher ends up polling repos that belong to other clusters which are never displayed since list never returns them. It should use the same filtered set that /func/list computes, either by accepting a repo list from the caller or by applying the same CLUSTER_API_URL variable check here.

Here is the logic of exclusion in the list endpoint - https://github.com/openshift/faas-console-plugin/blob/master/backend/handler/list.go#L151-L161

[matejvasek]

Surely should be fixed, but it's not critical, worst case: I get build statuses for a functions what we are not interested in on this specific cluster.

[matejvasek]

Right?

[pmeida]

Sure. Not critical. But should be done. I am fine with a follow up ticket.

[matejvasek]

@twoGiants I pushed the Ticker refactor.

[pmeida]

The SSE watcher does rediscover new repos every 30s, but that data is only merged onto rows already in functionItems - so it's orphaned until listFunctions re-runs. A browser refresh reconnects SSE from scratch anyway, making the rediscovery interval effectively invisible to the user. Its only practical effect today is pruning build status for repos that were removed.

Not auto-discovering functions created by other users is acceptable UX - a manual refresh to sync is reasonable. The issue is the naming: "rediscovery" implies new-repo detection, but in practice it only handles removals.

Wanted to leave this note. Not sure if we can do something now. Maybe it can originate a follow up.

[matejvasek]

/test e2e-aws

[matejvasek]

/test e2e-aws

[matejvasek]

/test e2e-aws

[openshift-ci]

@matejvasek: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/lint	78af5d8	link	true	/test lint

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.