From a8097b88a839128e52dfb8c784e6638f638496f2 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 16 Jul 2026 12:18:42 -0400 Subject: [PATCH] ENG-1940: Finish an in-progress build when the build-log stream is unavailable When the build-log-streamer WebSocket is denied for an in-progress build, complete the build without it: poll the build to completion, read the resolved artifact download URLs from a re-fetched commit, and drive the existing download/install path off them. A build that fails during polling is surfaced as a failure. The runtime gains a WithBuildPlanPoller option -- a caller-supplied closure, so pkg/runtime takes no buildplanner dependency -- which the runbits layer wires for in-progress builds. Together with the graceful-degradation change this sits on, a denied stream no longer stops state checkout / install from completing. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/runbits/runtime/runtime.go | 12 +++++ pkg/runtime/options.go | 6 +++ pkg/runtime/setup.go | 65 +++++++++++++++++++++---- pkg/runtime/setup_denial_test.go | 73 +++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 8 deletions(-) create mode 100644 pkg/runtime/setup_denial_test.go diff --git a/internal/runbits/runtime/runtime.go b/internal/runbits/runtime/runtime.go index dcdf29b446..c5b3c20ef8 100644 --- a/internal/runbits/runtime/runtime.go +++ b/internal/runbits/runtime/runtime.go @@ -283,6 +283,18 @@ func Update( q.Set("commitID", commitID.String()) u.RawQuery = q.Encode() rtOpts = append(rtOpts, runtime.WithBuildProgressUrl(u.String())) + // Fallback for when the build-log stream is unavailable. + rtOpts = append(rtOpts, runtime.WithBuildPlanPoller(func() (*buildplan.BuildPlan, error) { + bpm := bpModel.NewBuildPlannerModel(prime.Auth(), prime.SvcModel()) + if err := bpm.WaitForBuild(commitID, proj.Owner(), proj.Name(), nil); err != nil { + return nil, errs.Wrap(err, "Could not wait for the in-progress build to complete") + } + c, err := bpm.FetchCommit(commitID, proj.Owner(), proj.Name(), nil) + if err != nil { + return nil, errs.Wrap(err, "Could not fetch the completed build plan") + } + return c.BuildPlan(), nil + })) } if proj.IsPortable() { rtOpts = append(rtOpts, runtime.WithPortable()) diff --git a/pkg/runtime/options.go b/pkg/runtime/options.go index 63431b74a7..726b86a12a 100644 --- a/pkg/runtime/options.go +++ b/pkg/runtime/options.go @@ -1,6 +1,7 @@ package runtime import ( + "github.com/ActiveState/cli/pkg/buildplan" "github.com/ActiveState/cli/pkg/runtime/events" "github.com/go-openapi/strfmt" ) @@ -15,6 +16,11 @@ func WithAuthToken(token string) SetOpt { return func(opts *Opts) { opts.AuthToken = token } } +// WithBuildPlanPoller polls for a buildplan when the build-log stream is unavailable. +func WithBuildPlanPoller(poll func() (*buildplan.BuildPlan, error)) SetOpt { + return func(opts *Opts) { opts.PollBuildPlan = poll } +} + // WithDecryptionKey supplies the organization AES-256 key (and its id) used to // decrypt private artifacts during install. func WithDecryptionKey(key []byte, keyID string) SetOpt { diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index ded39d5324..abd21f6d78 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -62,6 +62,9 @@ type Opts struct { // the server can authorize the stream. Empty for unauthenticated callers. AuthToken string + // PollBuildPlan waits for an in-progress build when the build-log stream is unavailable. + PollBuildPlan func() (*buildplan.BuildPlan, error) + // OrgKey is the organization AES-256 key used to decrypt private artifacts // during install, with OrgKeyID identifying which key it is. Both are empty // when the runtime has no private ingredients. @@ -304,15 +307,12 @@ func (s *setup) update() error { // Wait for build to finish if !s.buildplan.IsBuildReady() && len(s.toBuild) > 0 { if err := blog.Wait(context.Background()); err != nil { - if buildlogstream.IsStreamDenied(err) { - if s.opts.AuthToken == "" { - return locale.WrapExternalError(err, "err_buildlog_stream_denied_unauthenticated", - "Could not monitor in-progress build. Please authenticate by running '[ACTIONABLE]state auth[/RESET]' and try again.") - } - return locale.WrapExternalError(err, "err_buildlog_stream_denied_unauthorized", - "Could not monitor in-progress build. If this is a private project, make sure your account has access to it.") + if !buildlogstream.IsStreamDenied(err) { + return errs.Wrap(err, "errors occurred during buildlog streaming") + } + if err := s.completeWithoutStream(wp); err != nil { + return err } - return errs.Wrap(err, "errors occurred during buildlog streaming") } } @@ -357,6 +357,55 @@ func (s *setup) update() error { return nil } +// completeWithoutStream finishes an in-progress build without the build-log +// stream: it polls the build to completion, reads the resolved artifact +// download URLs, and drives the normal download/install path off them. +func (s *setup) completeWithoutStream(wp *workerpool.WorkerPool) error { + if s.opts.PollBuildPlan == nil { + return errs.New("no build plan poller configured") + } + + logging.Debug("completing the in-progress build without the build-log stream") + resolved, err := s.opts.PollBuildPlan() + if err != nil { + return errs.Wrap(err, "Could not complete the in-progress build without the build-log stream") + } + + toObtain, err := s.resolveDownloads(resolved.Artifacts().ToIDMap()) + if err != nil { + return err + } + for _, a := range toObtain { + wp.Submit(func() error { + if err := s.obtain(a); err != nil { + return errs.Wrap(err, "obtain failed") + } + return nil + }) + } + return nil +} + +// resolveDownloads dresses each still-building artifact with the download URL +// from the completed build plan and returns the artifacts to obtain. Artifacts +// that weren't waiting on the build are left untouched (they were obtained +// already). It errors if a still-building artifact has no resolved URL. +func (s *setup) resolveDownloads(resolved buildplan.ArtifactIDMap) ([]*buildplan.Artifact, error) { + var toObtain []*buildplan.Artifact + for _, a := range s.toUnpack { + if _, building := s.toBuild[a.ArtifactID]; !building { + continue + } + ra, ok := resolved[a.ArtifactID] + if !ok || ra.URL == "" { + return nil, errs.New("completed build plan is missing a download URL for artifact %s", a.ArtifactID.String()) + } + a.SetDownload(ra.URL, ra.Checksum) + toObtain = append(toObtain, a) + } + return toObtain, nil +} + func (s *setup) onArtifactBuildReady(blog *buildlog.BuildLog, artifact *buildplan.Artifact, cb func()) { if _, ok := s.toBuild[artifact.ArtifactID]; !ok { // No need to build, artifact can already be downloaded diff --git a/pkg/runtime/setup_denial_test.go b/pkg/runtime/setup_denial_test.go new file mode 100644 index 0000000000..07ab214010 --- /dev/null +++ b/pkg/runtime/setup_denial_test.go @@ -0,0 +1,73 @@ +package runtime + +import ( + "strings" + "testing" + + "github.com/ActiveState/cli/internal/chanutils/workerpool" + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/pkg/buildplan" + "github.com/go-openapi/strfmt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestResolveDownloads verifies that the non-stream fallback copies download +// URLs from the completed build plan onto the artifacts that were waiting on +// the build, and leaves already-downloadable artifacts alone. +func TestResolveDownloads(t *testing.T) { + building := strfmt.UUID("11111111-1111-1111-1111-111111111111") + notBuilding := strfmt.UUID("22222222-2222-2222-2222-222222222222") + + buildingArt := &buildplan.Artifact{ArtifactID: building} + notBuildingArt := &buildplan.Artifact{ArtifactID: notBuilding} + + s := &setup{ + toUnpack: buildplan.ArtifactIDMap{building: buildingArt, notBuilding: notBuildingArt}, + toBuild: buildplan.ArtifactIDMap{building: buildingArt}, + } + + resolved := buildplan.ArtifactIDMap{ + building: &buildplan.Artifact{ArtifactID: building, URL: "https://dl/building", Checksum: "sha256:abc"}, + notBuilding: &buildplan.Artifact{ArtifactID: notBuilding, URL: "https://dl/other"}, + } + + toObtain, err := s.resolveDownloads(resolved) + require.NoError(t, err) + + require.Len(t, toObtain, 1, "only the still-building artifact needs obtaining") + assert.Equal(t, building, toObtain[0].ArtifactID) + assert.Equal(t, "https://dl/building", buildingArt.URL, "building artifact must get its resolved download URL") + assert.Equal(t, "sha256:abc", buildingArt.Checksum) + assert.Empty(t, notBuildingArt.URL, "an artifact that wasn't being built must be left untouched") +} + +// TestResolveDownloads_MissingURL verifies that a completed build plan missing a +// still-building artifact's URL is an error rather than a silent no-download. +func TestResolveDownloads_MissingURL(t *testing.T) { + building := strfmt.UUID("11111111-1111-1111-1111-111111111111") + buildingArt := &buildplan.Artifact{ArtifactID: building} + + s := &setup{ + toUnpack: buildplan.ArtifactIDMap{building: buildingArt}, + toBuild: buildplan.ArtifactIDMap{building: buildingArt}, + } + resolved := buildplan.ArtifactIDMap{building: &buildplan.Artifact{ArtifactID: building}} // no URL + + _, err := s.resolveDownloads(resolved) + require.Error(t, err) +} + +// TestCompleteWithoutStream_PollError verifies that a build that fails while +// polling (surfaced by the poller) is reported as a failure, not swallowed. +func TestCompleteWithoutStream_PollError(t *testing.T) { + s := &setup{opts: &Opts{ + PollBuildPlan: func() (*buildplan.BuildPlan, error) { + return nil, errs.New("build failed while polling") + }, + }} + err := s.completeWithoutStream(workerpool.New(1)) + require.Error(t, err) + assert.Contains(t, strings.ToLower(errs.JoinMessage(err)), "build failed while polling", + "the underlying build failure must be preserved") +}