fix(submissions): surface each linked build's outcome to the submitter (72c4aa38) · Commits · FelisMC / Felis · GitLab

docs/openapi.yaml

+18 −2

Changes for docs/openapi.yaml: 18 added lines, 2 removed lines.
Original line number	Diff line number	Diff line
		@@ -337,6 +337,19 @@ components:
		build_id:
		type: string
		description: image_builds.id, set only after the build hand-off succeeds.
		build_status:
		type: string
		enum: [pending, building, succeeded, failed, cancelled]
		description: >-
		The linked build's outcome, attached by the LIST routes
		(/me/submissions, /submissions) — for a submitter this is the only
		visible outlet for a failed build. Omitted until a build is linked
		and its row is readable.
		build_error:
		type: string
		description: >-
		The build's recorded failure text (e.g. a CRITICAL CVE scan
		failure), attached alongside build_status.
		reviewed_by: { type: string }
		reject_reason: { type: string }
		created_at: { type: string, format: date-time }
		@@ -4231,13 +4244,16 @@ paths:
		get:
		tags: [submissions]
		operationId: mySubmissions
		summary: List the caller's own modpack submissions (user-directed lane over §16).
		summary: List the caller's own modpack submissions with each linked build's outcome (user-directed lane over §16).
		x-felis-face: [external]
		x-felis-tier: app
		security: [{ accessJWT: [] }]
		responses:
		'200':
		description: The caller's submissions, newest first.
		description: >-
		The caller's submissions, newest first; rows with a linked build
		additionally carry build_status/build_error so the submitter can see
		whether their build succeeded or failed (and why).
		content:
		application/json:
		schema:

internal/api/images.go

+5 −0

Changes for internal/api/images.go: 5 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -15,6 +15,11 @@ import (
		// admin-tier (Zero Trust), enforced by adminOnly before these handlers run.
		type ImageBuilder interface {
		Submit(ctx context.Context, req build.Request) (*build.Build, error)
		// Get reads one build row without reconciling it — the read-only lookup the
		// submission views use to surface a build's outcome to its submitter (the
		// /images/build routes are admin-tier). State advance belongs to the
		// reconcile loop (Sync/SyncAll), so a list render never touches the cluster.
		Get(ctx context.Context, id string) (*build.Build, error)
		// Sync reconciles a build against its Job and returns the current view, so a
		// GET doubles as the reconcile tick (idempotent on terminal builds).
		Sync(ctx context.Context, id string) (*build.Build, error)

internal/api/images_test.go

+13 −0

Changes for internal/api/images_test.go: 13 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -16,6 +16,8 @@ import (
		type fakeBuilder struct {
		submitted *build.Request
		submitErr error
		getBuilds map[string]*build.Build
		getErr error
		syncErr error
		cancelErr error
		addedRef string
		@@ -40,6 +42,17 @@ func (f fakeBuilder) Submit(_ context.Context, req build.Request) (build.Build
		RequestedBy: req.RequestedBy}, nil
		}

		func (f fakeBuilder) Get(_ context.Context, id string) (build.Build, error) {
		f.lastBuildID = id
		if f.getErr != nil {
		return nil, f.getErr
		}
		if b, ok := f.getBuilds[id]; ok {
		return b, nil
		}
		return nil, build.ErrNotFound
		}

		func (f fakeBuilder) Sync(_ context.Context, id string) (build.Build, error) {
		f.lastBuildID = id
		if f.syncErr != nil {

internal/api/submissions.go

+55 −4

Changes for internal/api/submissions.go: 55 added lines, 4 removed lines.
Original line number	Diff line number	Diff line
		@@ -6,6 +6,7 @@ import (
		"io"
		"net/http"

		"felis.lolicon.best/internal/build"
		"felis.lolicon.best/internal/submit"
		)

		@@ -118,7 +119,9 @@ func (a API) handleUploadSubmissionContext(w http.ResponseWriter, r http.Reque

		// handleMySubmissions lists the caller's own submissions (app-tier). It scopes
		// strictly to the principal's id; there is no parameter that could widen the
		// query to another user's uploads.
		// query to another user's uploads. Each row is enriched with its linked build's
		// outcome — this list is the only player-visible outlet for a build result, so
		// a failed build is not invisible to the person who submitted it.
		func (a API) handleMySubmissions(w http.ResponseWriter, r http.Request) {
		if a.Submissions == nil {
		writeError(w, r, errSubmissionsUnavailable)
		@@ -130,12 +133,18 @@ func (a API) handleMySubmissions(w http.ResponseWriter, r http.Request) {
		writeSubmitError(w, r, err)
		return
		}
		writeJSON(w, http.StatusOK, map[string]any{"submissions": subs})
		views, err := a.submissionViews(r.Context(), subs)
		if err != nil {
		writeSubmitError(w, r, err)
		return
		}
		writeJSON(w, http.StatusOK, map[string]any{"submissions": views})
		}

		// handleListSubmissions is the admin review queue: every submission across all
		// users, newest first (admin-tier — it reads other users' uploads, so it gates
		// on the admin Zero-Trust path via adminOnly).
		// on the admin Zero-Trust path via adminOnly). Rows carry the same build
		// outcome enrichment as /me/submissions.
		func (a API) handleListSubmissions(w http.ResponseWriter, r http.Request) {
		if a.Submissions == nil {
		writeError(w, r, errSubmissionsUnavailable)
		@@ -146,7 +155,49 @@ func (a API) handleListSubmissions(w http.ResponseWriter, r http.Request) {
		writeSubmitError(w, r, err)
		return
		}
		writeJSON(w, http.StatusOK, map[string]any{"submissions": subs})
		views, err := a.submissionViews(r.Context(), subs)
		if err != nil {
		writeSubmitError(w, r, err)
		return
		}
		writeJSON(w, http.StatusOK, map[string]any{"submissions": views})
		}

		// submissionView is one submission row enriched with its linked build's
		// outcome. The row itself is embedded unchanged, so the wire shape only gains
		// the two optional fields; they appear solely once a build has been linked
		// (BuildID set) and its record is still readable.
		type submissionView struct {
		submit.Submission
		BuildStatus string `json:"build_status,omitempty"`
		BuildError string `json:"build_error,omitempty"`
		}

		// submissionViews enriches each submission with its linked build's status via a
		// read-only Builder.Get — deliberately never Sync, because the 15s reconcile
		// loop owns state advance and rendering a list must not touch the cluster. A
		// submission with no linked build (never approved, or approved before the
		// hand-off could record the id), no Builder wired, or a build row that is gone
		// (ErrNotFound) renders without the extra fields; any other store failure is
		// returned so the handler reports it rather than silently dropping the outcome.
		func (a *API) submissionViews(ctx context.Context, subs []submit.Submission) ([]submissionView, error) {
		views := make([]submissionView, len(subs))
		for i, s := range subs {
		views[i] = submissionView{Submission: s}
		if a.Builder == nil \|\| s.BuildID == "" {
		continue
		}
		bld, err := a.Builder.Get(ctx, s.BuildID)
		if errors.Is(err, build.ErrNotFound) {
		continue
		}
		if err != nil {
		return nil, err
		}
		views[i].BuildStatus = string(bld.Status)
		views[i].BuildError = bld.Error
		}
		return views, nil
		}

		// handleApproveSubmission is the admin approve gate (admin-tier). The reviewer is

internal/api/submissions_test.go

+71 −1

Changes for internal/api/submissions_test.go: 71 added lines, 1 removed line.
Original line number	Diff line number	Diff line
		@@ -10,6 +10,7 @@ import (
		"strings"
		"testing"

		"felis.lolicon.best/internal/build"
		"felis.lolicon.best/internal/submit"
		)

		@@ -270,6 +271,68 @@ func TestMySubmissionsScopesToPrincipal(t *testing.T) {
		}
		}

		// The "my uploads" list carries the linked build's outcome — for a submitter it
		// is the only visible outlet for a failed build (the /images/build routes are
		// admin-tier). A row with no linked build gains no build fields.
		func TestMySubmissionsCarriesBuildOutcome(t *testing.T) {
		fs := &fakeSubmissions{byResult: []submit.Submission{
		{ID: "sub-1", SubmittedBy: "user-7", Status: submit.StatusApproved, BuildID: "bld-9"},
		{ID: "sub-2", SubmittedBy: "user-7", Status: submit.StatusPendingReview},
		}}
		api := appSubAPI(fs)
		api.Builder = &fakeBuilder{getBuilds: map[string]*build.Build{
		"bld-9": {ID: "bld-9", Status: build.StatusFailed, Error: "build job failed or scan found a CRITICAL CVE"},
		}}
		w := do(api.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil)
		if w.Code != http.StatusOK {
		t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
		}
		var got struct {
		Submissions []struct {
		ID string `json:"id"`
		BuildStatus string `json:"build_status"`
		BuildError string `json:"build_error"`
		} `json:"submissions"`
		}
		if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
		t.Fatalf("body not JSON: %v", err)
		}
		if len(got.Submissions) != 2 {
		t.Fatalf("submissions = %d, want 2", len(got.Submissions))
		}
		if got.Submissions[0].BuildStatus != "failed" \|\| got.Submissions[0].BuildError == "" {
		t.Errorf("sub-1 outcome = %+v, want failed with the error text", got.Submissions[0])
		}
		if got.Submissions[1].BuildStatus != "" \|\| got.Submissions[1].BuildError != "" {
		t.Errorf("sub-2 outcome = %+v, want no build fields without a linked build", got.Submissions[1])
		}
		}

		// A linked build whose row is gone renders as "no outcome" rather than failing
		// the whole list; any other lookup failure must surface, never be swallowed.
		func TestMySubmissionsBuildLookupSemantics(t *testing.T) {
		// Missing row (ErrNotFound): 200 with no build fields.
		fs := &fakeSubmissions{byResult: []submit.Submission{{ID: "sub-1", BuildID: "bld-gone"}}}
		api := appSubAPI(fs)
		api.Builder = &fakeBuilder{}
		w := do(api.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil)
		if w.Code != http.StatusOK {
		t.Fatalf("missing build row: code = %d, want 200 (%s)", w.Code, w.Body.String())
		}
		if strings.Contains(w.Body.String(), "build_status") {
		t.Errorf("missing build row: body carries build fields: %s", w.Body.String())
		}

		// Store fault: the failure is reported, not hidden behind a 200.
		fs = &fakeSubmissions{byResult: []submit.Submission{{ID: "sub-1", BuildID: "bld-1"}}}
		api = appSubAPI(fs)
		api.Builder = &fakeBuilder{getErr: errors.New("db down")}
		w = do(api.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil)
		if w.Code != http.StatusInternalServerError {
		t.Fatalf("store fault: code = %d, want 500 (%s)", w.Code, w.Body.String())
		}
		}

		// Every /submissions route is admin-tier: a plain user is rejected before the
		// handler runs.
		func TestSubmissionAdminRoutesAreAdminOnly(t *testing.T) {
		@@ -294,9 +357,12 @@ func TestSubmissionAdminRoutesAreAdminOnly(t *testing.T) {
		func TestListSubmissionsAdmin(t *testing.T) {
		fs := &fakeSubmissions{listed: []submit.Submission{
		{ID: "sub-1", SubmittedBy: "user-7", Status: submit.StatusPendingReview},
		{ID: "sub-2", SubmittedBy: "user-9", Status: submit.StatusApproved},
		{ID: "sub-2", SubmittedBy: "user-9", Status: submit.StatusApproved, BuildID: "bld-2"},
		}}
		api := adminSubAPI(fs)
		api.Builder = &fakeBuilder{getBuilds: map[string]*build.Build{
		"bld-2": {ID: "bld-2", Status: build.StatusSucceeded},
		}}
		w := do(api.ExternalHandler(), "GET", "/api/v1/submissions", "", nil)
		if w.Code != http.StatusOK {
		t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
		@@ -308,6 +374,10 @@ func TestListSubmissionsAdmin(t *testing.T) {
		if len(got["submissions"]) != 2 {
		t.Fatalf("submissions = %d, want 2", len(got["submissions"]))
		}
		// The admin queue carries the same build outcome enrichment.
		if !strings.Contains(w.Body.String(), `"build_status":"succeeded"`) {
		t.Errorf("admin queue lacks the linked build outcome: %s", w.Body.String())
		}
		}

		// The reviewer is the admin principal's email, never client input.