From 33a0b2ecd5bfde9b516a80c6fc30b44682134821 Mon Sep 17 00:00:00 2001 From: jsmith212 <29954021+jsmith212@users.noreply.github.com> Date: Thu, 3 Sep 2026 11:54:48 -0600 Subject: [PATCH 1/3] fix(api): never send push_commands as null; guard it in the console Selecting any tile but yocto on the snippets screen threw `TypeError: can't access property "length", ... push_commands is null` and left the skeleton placeholder up forever. Every non-yocto builder -- and yocto on a project without sstate/downloads -- leaves the PushCommands slice nil, which encoding/json marshals as `null`, and the page read `.length` off it unguarded (files/env were already guarded). - snippetResponse normalises nil to an empty slice, so push_commands is an array on the wire for every tool and both the minted and preview branches (same convention as newProject's backends). - The page reads push_commands through a `?? []` derived, like files and env, and the "nothing to show" empty state now names the backend kind the selected tool needs and links to the project's backends page. - TestSnippetPushCommandsIsAnArrayOnTheWire asserts the JSON body, not the decoded struct: `len(nil) == 0` let every existing len-based check pass while the wire carried null. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NonmtCqhSy8f13VcK1rBMb --- internal/api/snippets.go | 14 ++++++- internal/api/snippets_test.go | 42 +++++++++++++++++++ .../o/[org]/p/[project]/snippets/+page.svelte | 23 +++++++--- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/internal/api/snippets.go b/internal/api/snippets.go index e3be6ee..8bef4f0 100644 --- a/internal/api/snippets.go +++ b/internal/api/snippets.go @@ -351,13 +351,25 @@ func (a *API) handleGenerateSnippet(w http.ResponseWriter, r *http.Request) erro // snippetResponse assembles the wire body. key == nil is a PREVIEW: 200, no // credential, snippetTokenPlaceholder wherever a token would be. func snippetResponse(t snippetTarget, c snippetContent, key *CreatedAPIKey) SnippetResponse { + // push_commands is an ARRAY on the wire, always -- never null. It is legitimately + // empty for eight of the nine tools (they write to the cache themselves) and for + // a yocto project with neither sstate nor downloads, and every builder leaves it + // nil in those cases. A nil slice marshals as `null`, the console reads + // `.length` off it and throws before it can render the warnings that say WHY + // the snippet is thin -- a blank screen where the explanation should be. Same + // convention as newProject's backends. + push := c.pushCommands + if push == nil { + push = []string{} + } + return SnippetResponse{ Tool: t.tool, Host: hostOnly(t.host), BaseURL: t.baseURL, LocalConf: c.localConf, Netrc: c.netrc, - PushCommands: c.pushCommands, + PushCommands: push, Files: c.files, Env: c.env, APIKey: key, diff --git a/internal/api/snippets_test.go b/internal/api/snippets_test.go index ea7c7c5..725ed9d 100644 --- a/internal/api/snippets_test.go +++ b/internal/api/snippets_test.go @@ -1185,6 +1185,48 @@ func TestSnippetOCIToolsHaveNoPushCommands(t *testing.T) { } } +// TestSnippetPushCommandsIsAnArrayOnTheWire pins the JSON shape, not the Go one: +// `len(nil) == 0` passes every len-based assertion in this file while the wire +// carries `"push_commands":null`, and the console does `.length` on it -- which +// is how selecting ANY non-yocto tile blanked the snippets screen in v0.1.0. The +// three cases are the three ways the slice is left nil: a tool that never has a +// push, an OCI tool (pull-through only), and yocto on a project with no sstate or +// downloads. Both the minted and the preview branch go through snippetResponse. +func TestSnippetPushCommandsIsAnArrayOnTheWire(t *testing.T) { + const wantKey = `"push_commands":[]` + + cases := []struct { + name string + store Store + body string + want int + }{ + {"bazel minted", snippetStore(t, allSnippetBackends...), `{"tool":"bazel","scope":"read"}`, http.StatusCreated}, + {"bazel preview", snippetStore(t, allSnippetBackends...), `{"tool":"bazel","preview":true}`, http.StatusOK}, + {"docker preview", snippetStore(t, allSnippetBackends...), `{"tool":"docker","preview":true}`, http.StatusOK}, + {"yocto with no backends", snippetStore(t), `{"tool":"yocto","preview":true}`, http.StatusOK}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + a := snippetAPI(t, tc.store, &fakeMinter{token: snippetToken}) + w := snippetPost(t, a, principals(t)["proj_write"], tc.body, nil) + + if w.Code != tc.want { + t.Fatalf("status = %d, want %d: %s", w.Code, tc.want, w.Body.String()) + } + + if !strings.Contains(w.Body.String(), wantKey) { + t.Errorf("wire body must carry %s, got:\n%s", wantKey, w.Body.String()) + } + + if strings.Contains(w.Body.String(), `"push_commands":null`) { + t.Errorf("push_commands must never be null on the wire") + } + }) + } +} + // TestSnippetNeverEmitsAnIDSecretPair: a Bakery cache credential is ONE opaque // bkry_ token. `key_id:key_secret` is a credential that cannot exist, and it was the // single most repeated drift in the hand-authored console copy this generator diff --git a/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte b/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte index e67d73e..d728013 100644 --- a/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte +++ b/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte @@ -142,6 +142,11 @@ } const gotcha = $derived(gotchas[selected]); + const selectedTile = $derived(tiles.find((t) => t.id === selected) ?? tiles[0]); + // The server guarantees an array, but `.length` on a missing field is how this + // screen blanked in v0.1.0 (nil slice -> `null`), so read it defensively -- + // same as `files` and `env` below. + const pushCommands = $derived(response?.push_commands ?? []); {#snippet block(title: string, content: string)} @@ -252,8 +257,8 @@ {#if response.netrc} {@render block('~/.netrc', response.netrc)} {/if} - {#if response.push_commands.length > 0} - {@render block('after a build — push to the cache', response.push_commands.join('\n'))} + {#if pushCommands.length > 0} + {@render block('after a build — push to the cache', pushCommands.join('\n'))} {/if} {#each response.files ?? [] as f (f.path)}
@@ -267,12 +272,18 @@ response.env.map((e) => `export ${e.name}="${e.value}"`).join('\n') )} {/if} - {#if !response.local_conf && !response.netrc && response.push_commands.length === 0 && (!response.files || response.files.length === 0) && (!response.env || response.env.length === 0)} + {#if !response.local_conf && !response.netrc && pushCommands.length === 0 && (!response.files || response.files.length === 0) && (!response.env || response.env.length === 0)} + title="Nothing to generate for {selectedTile.name}" + desc="{selectedTile.name} needs a {selectedTile.backend} backend on this project, and there isn't one. Add it, then pick this tile again — the warnings above say exactly what was left out." + > + {#snippet action()} + + {/snippet} + {/if} {/if}
From 6e06eea440fd6ea48a8d46b3b5c5e88d89a5f1b7 Mon Sep 17 00:00:00 2001 From: jsmith212 <29954021+jsmith212@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:18:54 -0600 Subject: [PATCH 2/3] fix(web): repair the snippet empty state's CTA, copy, and mint gating Review findings on the parent commit, all in the reworked empty state: - The "Configure backends" button linked /o/{org}/p/{project}/backends, a route that does not exist -- the remediation CTA landed on the not-found page. Now projectPath(...)/backends/new, the same target the overview page uses (ConsoleNav documents this exact trap). - The copy asserted "there isn't one. Add it" for a state also reached when the backend exists but is disabled -- following it earned a 409 from the (project_id, kind) unique. The copy now defers to the warnings, which carry the missing-vs-disabled distinction. - The mint bar is now gated on the preview having content: minting on a backend-less tile created a real one-time secret with no blocks to bake it into, rendered above an empty state whose advice (pick the tile again) would wipe it. hasContent is one shared derivation, so the mint bar and the empty state cannot disagree. - "a oci backend" -> "the oci backend" on four of nine tiles. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YM78iXWsYNLXccdzKhQcx1 --- .../o/[org]/p/[project]/snippets/+page.svelte | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte b/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte index d728013..6fa0fd1 100644 --- a/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte +++ b/web/src/routes/(console)/o/[org]/p/[project]/snippets/+page.svelte @@ -1,5 +1,6 @@ {#snippet block(title: string, content: string)} @@ -212,7 +225,7 @@ {/if} - {#if response.preview} + {#if response.preview && hasContent}
@@ -272,14 +285,17 @@ response.env.map((e) => `export ${e.name}="${e.value}"`).join('\n') )} {/if} - {#if !response.local_conf && !response.netrc && pushCommands.length === 0 && (!response.files || response.files.length === 0) && (!response.env || response.env.length === 0)} + {#if !hasContent} {#snippet action()} - {/snippet} From 314d7a86a975e4db362e88f1ddbc45f788aaeb41 Mon Sep 17 00:00:00 2001 From: jsmith212 <29954021+jsmith212@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:18:54 -0600 Subject: [PATCH 3/3] fix(e2e): scope the live-badge assertion to the minted token's row A bare getByText('live') matches every live token's status badge; the seeded dev user's table holds one per prior run against a reused local database, so the assertion is a strict-mode violation everywhere but CI's fresh database. Same ambiguous-locator class as the robot-row fix (5ed6dcd), same remedy: scope to the row the test just created. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YM78iXWsYNLXccdzKhQcx1 --- web/e2e/console.spec.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/web/e2e/console.spec.ts b/web/e2e/console.spec.ts index af7f0f7..cf3ebde 100644 --- a/web/e2e/console.spec.ts +++ b/web/e2e/console.spec.ts @@ -316,8 +316,14 @@ test.describe('console: personal access tokens and robots (wave 1)', () => { }); await test.step('the token now appears live in the table', async () => { - await expect(page.getByText(tokenName)).toBeVisible(); - await expect(page.getByText('live')).toBeVisible(); + // Scope to the row this test just created: a bare getByText('live') + // matches every live token's status badge, and the seeded dev user's + // table is not guaranteed to hold exactly one -- a reused local + // database carries rows from earlier runs (CI's fresh database + // masks the ambiguity). Same class as the robot-row locator fix. + const row = page.getByRole('row').filter({ hasText: tokenName }); + await expect(row).toBeVisible(); + await expect(row.getByText('live')).toBeVisible(); }); });