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]

@pmeida @dsimansk plz review backend

[matejvasek]

@twoGiants docs/plans/completed/2026-08-26-SRVOCF-1038-build-status.md has 1748 lines and account for 39% of this PR. Question is do we want plans committed?

Production code is about 750 lines. Tests are about 1500 lines.

[matejvasek]

/test all

[matejvasek]

Self-review note on this signature, error handling specifically. Weak suggestion, not necessary for this PR, filing it so the reasoning is recorded rather than lost.

WatchWorkflowRuns(ctx context.Context, workflowFile string) (<-chan []RepoRun, error)

The setup/stream split is right: repo discovery is synchronous, so HandleBuildWatch can still return a real 401 before the response turns into SSE. The weaker end is termination. Closing the channel is the only signal, and it carries one bit.

Three different things close it:

• ctx cancelled, i.e. the browser disconnected (normal)
• the token was revoked mid-stream (scm/github/watch.go:87)
• any terminal producer error we add later

The consumer cannot tell them apart, which is why handler/build.go:85 has to hedge: "Watch ended (cancelled, or the token was revoked mid-stream)." That is fine today because both paths end the SSE stream, but the handler cannot log the difference, and a future terminal error would be indistinguishable from a clean shutdown.

If we ever want to distinguish them, abort would stay ctx-only (deliberately no Stop(), so there is no "who wins" question and no drain-to-avoid-deadlock contract):

// RunWatch carries a stream of workflow-run snapshots. C is closed when the
// watch ends; Err then reports why, or nil if it ended cleanly because ctx
// was cancelled.
type RunWatch struct {
	C   <-chan []RepoRun
	err error
}

// Err is valid only after C is closed: the close is the happens-before edge
// that publishes err. Same contract as sql.Rows.Err.
func (w *RunWatch) Err() error { return w.err }

WatchWorkflowRuns(ctx context.Context, workflowFile string) (*RunWatch, error)

Producer sets err before close(out); consumer reads it only in the !ok branch:

case snapshot, ok := <-runs.C:
	if !ok {
		if err := runs.Err(); err != nil {
			slog.Info("build watch: stream ended", "err", err)
		}
		return
	}

Two notes on the shape:

• Err is a method rather than a field mainly so it could gain an atomic.Pointer[error] later without touching call sites, if the "only after close" contract turned out to be too sharp. As written it is no more synchronized than a field would be; the safety comes from the close, not from the accessor.
• Not generic. One instantiation, and *Watch[[]RepoRun] reads badly. Easy to promote later if a second watch appears.

The other common shape, an Event{Value, Err} channel, seems worse here: every consumer branches per element on a field set at most once, and you can still receive after the error unless you also close.

Separate and much smaller, same file: ClientStub.WatchWorkflowRuns returns (nil, nil) by default (client.go:170). A nil channel never fires in select, so a test that forgets to set OnWatchWorkflowRuns gets an SSE stream that heartbeats forever instead of failing. Returning a closed channel would surface the mistake immediately.

Neither is blocking. Happy to leave both for a follow-up.

[matejvasek]

/test all

[pmeida]

Also need a follow up on the previous comments:

• #177 (comment)
• #177 (comment)

[pmeida]

default namespace should not be used. Consider switching to PRESEEDED_FUNC_NAMESPACE.

It does not change much right now as we are faking everything, but in the future, after integrating act into the fakegithub, IMO this test should be adapted to full flow: create function, let the workflow run in act, verify Building, update to broken code in function, deploy it, verify...

[twoGiants]

@twoGiants docs/plans/completed/2026-08-26-SRVOCF-1038-build-status.md has 1748 lines and account for 39% of this PR. Question is do we want plans committed?

Production code is about 750 lines. Tests are about 1500 lines.

@matejvasek yes we commit the plans and designs. I want the AI contributions to this project be transparent and visible. Especially I want it to be visible how much review is needed for AI powered PRs.

Communicating that the plan is the biggest chunk of the PR already reduces PR review burden.

[twoGiants]

@twoGiants I think the FE is starting to become really messy and we should reconsider folder architecture ASAP. This way it is not scaling. Resolved

@Cragsmann lets connect and you show me exactly what you mean. I want to understand where you're seeing the mess and the scalability issues.

Keeping child components which are only used by one parent in the same module scales equally to keeping private methods of a class in that same class.

Please, lets align and clarify.

[pmeida]

as discussed in F2F we also need to change 3 icons, so that it is consistent with the rest of the platform and it doesn't repeat the same icon twice in the status column:

My suggestion:

secondary status:

• from dynamic spinner to a static one - pficon-in-progress

primary status:

• <icon> Building -> icon=fa-wrench
• <icon> Deploying -> icon=fa-cube

@twoGiants @matejvasek

[twoGiants]

First bigger batch of the FE review.

Great job overall as always. That was a lot of work, I appreciate it! Thank you for using the new testing patterns in the List page and in the hook tests. And thank you for creating the test doubles using the new style. It's cool that we're moving in this direction.

Now lets improve a few things:

• The hook can be written better testable and better usable for it's clients.
• The test double API can be simplified and the stub can become a fake.

I continue next week.

[twoGiants]

Pls remove all the zero value comments.

I'll tag them with "remove comment" from now on.

[twoGiants]

Use a concrete type.

unknown is as good as any type. Only use it if there is really no other way.

In the fixtures setup for useK8WatchResourceStub I was able to refactor towards known types everywhere.

[twoGiants]

This should be done via the context. This is for a follow up. We have a ticket for it https://redhat.atlassian.net/browse/SRVOCF-842. Can you add to the ticket that this hooks will need to be updated too.

[twoGiants]

Maybe name it streaming or running? Then the statements read a bit nicer down below e.g. while (streaming).

[twoGiants]

We log errors but do nothing about it. Same below.

I'd rather return the error like useCluster does and handle it in the client.

[twoGiants]

This is a contract test -> timeout must be 0 or it disconnects after 60s. It's important that we keep it. Just don't spy on internals. Use a fake implementation, then fake timers and assert on the result.

The type (no unknown):

type ConsoleFetch = (url: string, options?: RequestInit, timeout?: number) => Promise<Response>;

The stub:

export const consoleFetchStub = (
  url: string,
  options?: RequestInit,
  timeout?: number,
): Promise<Response> => {
  if (streamError) return Promise.reject(streamError);

  const stream = new ReadableStream<Uint8Array>({
    start(controller) {
      const encoder = new TextEncoder();
      for (const chunk of frames) {
        controller.enqueue(encoder.encode(chunk));
      }

      // Keep the stream open like a real SSE connection.
      // End it only when the caller aborts or the timeout fires.
      options?.signal?.addEventListener('abort', () => controller.close());

      if (timeout) {
        setTimeout(() => controller.error(new Error('timeout')), timeout);
      }
    },
  });
  return Promise.resolve(new Response(stream, { status: 200 }));
};

The test:

it('keeps the stream alive beyond the default consoleFetch timeout', async () => {
  vi.useFakeTimers();
  streamStub.setStreamFrames([
    streamStub.buildStatusFrame({ 'alice/fn': { buildStatus: 'Building' } }),
  ]);

  const { result, unmount } = renderHook(() => useBuildStatus());

  await waitFor(() => expect(result.current.size).toBe(1));

  // Advance past the default 60s consoleFetch timeout.
  // If the hook didn't disable it, the stream would be dead and data cleared on reconnect error.
  await vi.advanceTimersByTimeAsync(120_000);

  expect(result.current.get('alice/fn')?.buildStatus).toBe('Building');

  unmount();
});

[twoGiants]

Test on the visible behaviour and the result. I'd send an unnamed event followed by named event and only the named should appear in the results. With the refactored test, the proof that the unnamed frame was consumed is that the valid frame after it was processed.

it('ignores a frame with no event name', async () => {
  streamStub.setStreamFrames([
    'data: {"irrelevant":"not a build-status event"}\n\n',
    streamStub.buildStatusFrame({ 'c/d': { buildStatus: 'Succeeded' } }),
  ]);

  const { result } = renderHook(() => useBuildStatus());

  await waitFor(() => expect(result.current.size).toBe(1));
  expect(result.current.get('c/d')?.buildStatus).toBe('Succeeded');
});

[twoGiants]

You need two tests here, one for 401 and one for 403 and you must return error from the hook so that you can assert against it here. No spy needed then.

it('surfaces an auth error and stops the stream', async () => {
  streamStub.setStreamError(Object.assign(new Error('unauthorized'), { code: 401 }));

  const { result } = renderHook(() => useBuildStatus());

  await waitFor(() => expect(result.current.error).toBeTruthy());
  expect(result.current.statuses.size).toBe(0);
});

[twoGiants]

This doesn't look right. Any error except 401 and 403 is a reconnect. Are you sure about that? I would rather indicate somehow in the UI that we're reconnecting instead of doing it silently then we can also test easier against it.

So this should probably set some state which is returned from this hook like { reconnecting }.

[matejvasek]

Are you sure about that?

TBH I am not. I am only sure that with 401 and 403 there is no point in retry.

What error should be retryable? Connection dial error? 503 or any 5XX?

[twoGiants]

I'll stop for now. I'm not yet done with reviewing all the tests.

What is clear so far is that the useBuildStatus implementation is difficult to test, so you'll need to refactor it a bit.

Also the consoleFetchStub test double needs refactoring towards a fake and the spy API for consoleFetchStub needs to be removed. After a refactoring of useBuildStatus you won't need it. You should always be able to test against observable behaviour.

[matejvasek]

/test all

[matejvasek]

/test e2e-aws

[twoGiants]

Now I understood useBuildStatus. Lets separate the concerns there in a clean way and then we're good. You're very close! Great job on the low level implementaton and great job on putting everything close together -> the extraction will be simple because you prepared it so well. 👍

I'll finish the tests next time I review.

[twoGiants]

This is not needed, use Record<string, BuildStatus>. The BuildSnapshot already comes in the shape we can work with it. In FunctionListPage.tsx you use const build = buildStatuses.get(${item.owner}/${item.repoName}); but this can be done with a record too -> buildStatuses[key].

Then you use setStatuses(snap.functions) directly.

[twoGiants]

Now after a good night sleep I understand this much better. These two functions plus the consoleFetch are the low level implementation of the SSE receiving. If we keep SSE, we might reuse it in more client hooks, but no need to extract now, just refactor properly. (Or extract if you feel like it, it'll come anyway.)

Ideally this entire part would live in functionsClient.ts as an async function* receiveFunctionsBuildStatus() which after the useCluster to backend migration probably will become a receiveFunctionsStatus or even receiveFunctions. In functionsClient.ts is also the scmHeaders function which reads the pat. The receiving and deserialization bits would be unexported functions in that module, like now in this hook module.

There are three main things happening for the receiving implementation:

1. Setting up the stream via consoleFetch.
2. Receiving incoming data in chunks.
3. Deserializing.

I think they should all live in one generic generator function and be provided from functionsClient.ts via receiveFunctionsBuildStatus and consumed by useBuildStatus hook in the useEffect block. The deserialization should be generic, I wouldn't couple it to BuildSnapshot. I'd also pass in the event type as an argument.

// in functionsClient.ts
export async function* receiveFunctionsBuildStatus(
  abortSignal: AbortSignal,
): AsyncGenerator<BuildSnapshot> {
  yield* receiveSSE<BuildSnapshot>(
    `${PROXY_BASE}/api/v1/func/build/watch`,
    'build-status',
    abortSignal,
  );
}

async function* receiveSSE<T>(
  url: string,
  eventName: string,
  abortSignal: AbortSignal,
): AsyncGenerator<T> {
  const res = await consoleFetch(url, { headers: scmHeaders(), signal: abortSignal }, 0);
  if (!res.body) return;

  const decoder = new TextDecoder();
  let buffer = '';
  for await (const chunk of res.body) {
    buffer += decoder.decode(chunk, { stream: true });
    let idx: number;
    while ((idx = buffer.indexOf('\n\n')) !== -1) {
      const raw = buffer.slice(0, idx);
      buffer = buffer.slice(idx + 2);
      const parsed = deserializeSSEFrame(raw);
      if (parsed) yield parsed;
    }
  }

  function deserializeSSEFrame(frame: string): T | null {
    let event = '';
    const dataLines: string[] = [];
    for (const line of frame.split('\n')) {
      if (line.startsWith(':')) continue;
      if (line.startsWith('event:')) event = line.slice('event:'.length).trim();
      else if (line.startsWith('data:')) dataLines.push(line.slice('data:'.length).trim());
    }
    if (event !== eventName) return null;
    if (dataLines.length === 0) return null;
    try {
      return JSON.parse(dataLines.join('\n')) as T;
    } catch {
      return null;
    }
  }
}

The try block in useBuildStatus would be reduced to:

  useEffect(() => {
    let cancelled = false;
    const controller = new AbortController();

    async function run() {
      while (!cancelled) {
        const pat = sessionStorage.getItem(PAT_KEY);
        if (!pat) return;
        try {
          for await (const snap of receiveFunctionsBuildStatus(controller.signal)) {
            if (!cancelled) setStatuses(snap.functions);
          }
        } catch (err) {
          if (cancelled) return;
       //...

[matejvasek]

/test e2e-aws

[openshift-ci]

@matejvasek: all tests passed!

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.