From 695a1b2f2de6c8fc556ac43d96c3434f0378922c Mon Sep 17 00:00:00 2001 From: Artjoms Stukans Date: Sun, 27 Sep 2026 03:14:36 +0200 Subject: [PATCH 1/4] Fix the screens the first subscription build walked through What the operator hit on the first live subscription build: - Harness keys: a switched-off key or seat can now be deleted. One no run used is removed; one a run used has its secret erased and leaves the screen, keeping only the name the run shows (V86). Switching off alone kept the key or the person's sign-in stored for ever. - Build setup: the form says which fields keep it from saving, instead of a Save button that silently does nothing. - Work items: says that tickets arrive by a scheduled scan, so a new one can take a minute or two to appear. - Approval panel: shows how the build pays, and labels which text is the specification and which the one-step plan. - Work item page: reads the item again every 10 seconds while no panel is open, like Runs, so a gate or a finished build shows without a click. - Verify stop: says verification is not built yet and the result is held, with no push and no pull request. - Ticket source: names the account by role and bot handle. Two accounts shared one name, so the reviewer account had been picked unseen. --- .../factory/HarnessCredentialPool.java | 79 +++++++++- .../factory/HarnessCredentialResource.java | 19 +++ .../V86__harness_credential_delete.sql | 19 +++ .../factory/HarnessCredentialDeleteTest.java | 135 ++++++++++++++++++ spire-ui/src/api.ts | 7 + .../SettingsHarnessCredentials.test.tsx | 18 +++ .../components/SettingsHarnessCredentials.tsx | 29 +++- spire-ui/src/components/accounts.ts | 4 +- .../repositories/factory/BuildStep.tsx | 4 + .../factory/RepositoryFactory.test.tsx | 11 ++ .../repositories/factory/SourceStep.test.tsx | 15 +- .../repositories/factory/SourceStep.tsx | 7 +- .../work-items/DecisionEvidence.tsx | 9 +- .../work-items/DecisionPanel.test.tsx | 9 ++ .../components/work-items/WorkItemDetail.tsx | 15 ++ .../components/work-items/WorkItems.test.tsx | 19 +++ .../src/components/work-items/WorkItems.tsx | 2 + .../work-items/workPreparationApi.ts | 2 + .../src/components/work-items/workReasons.ts | 2 +- 19 files changed, 391 insertions(+), 14 deletions(-) create mode 100644 spire-orchestrator/src/main/resources/db/migration/V86__harness_credential_delete.sql create mode 100644 spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessCredentialDeleteTest.java diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java index 0095b14c..1709bd96 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialPool.java @@ -384,7 +384,7 @@ public List list() { String sql = """ SELECT id, label, type, base_url, enabled, rate_limited_until, rejected_at, last_used_at, auth_mode, auth_mode <> 'SUBSCRIPTION' OR account_ref IS NOT NULL AS identified - FROM harness_credential ORDER BY label + FROM harness_credential WHERE erased_at IS NULL ORDER BY label """; List members = new ArrayList<>(); try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement(sql); @@ -444,7 +444,7 @@ INSERT INTO harness_credential (id, label, type, base_url, api_key) * person to their phone. Finding out afterwards means a completed sign-in with nowhere to go. */ public boolean hasLabel(Connection c, String label) throws SQLException { - try (PreparedStatement ps = c.prepareStatement("SELECT 1 FROM harness_credential WHERE label = ?")) { + try (PreparedStatement ps = c.prepareStatement("SELECT 1 FROM harness_credential WHERE label = ? AND erased_at IS NULL")) { ps.setString(1, label); try (ResultSet rs = ps.executeQuery()) { return rs.next(); } } @@ -555,7 +555,7 @@ private int identifySeats(Connection c) throws SQLException { int switchedOff = 0; try (PreparedStatement ps = c.prepareStatement(""" SELECT id, type, api_key FROM harness_credential - WHERE auth_mode = 'SUBSCRIPTION' AND account_ref IS NULL ORDER BY updated_at DESC + WHERE auth_mode = 'SUBSCRIPTION' AND account_ref IS NULL AND erased_at IS NULL ORDER BY updated_at DESC """); ResultSet rs = ps.executeQuery()) { while (rs.next()) { UUID id = rs.getObject("id", UUID.class); @@ -607,10 +607,81 @@ public boolean remove(UUID id) { id) == 1; } + /** What {@link #delete} did. */ + public enum Deletion { + /** No run used it, so the row is gone. */ + DELETED, + /** A run used it: its secret is erased and it is hidden, but the row stays so the run can name it. */ + ERASED, + /** It is still switched on. Switch it off first, so no build is picking it while it goes. */ + STILL_ON, + NOT_FOUND + } + + /** + * Delete a switched-off member (operator feedback, 2026-09-27). + * + *

Switching off was the only way to remove one, and it kept the key or the person's sign-in file + * stored for ever. A member no run used is now deleted outright. One a run used cannot be — the run's + * attribution points at it — so its secret is erased and it leaves the pool screen, keeping only the + * name a finished run shows. Either way the secret is gone. + */ + public Deletion delete(UUID id) { + try (Connection c = dataSource.getConnection()) { + c.setAutoCommit(false); + try { + Deletion outcome = delete(c, id); + c.commit(); + return outcome; + } catch (SQLException | RuntimeException failure) { + c.rollback(); + throw failure; + } + } catch (SQLException e) { + throw new IllegalStateException("The harness credential " + id + " could not be deleted", e); + } + } + + private Deletion delete(Connection c, UUID id) throws SQLException { + try (PreparedStatement ps = c.prepareStatement( + "SELECT enabled FROM harness_credential WHERE id = ? AND erased_at IS NULL FOR UPDATE")) { + ps.setObject(1, id); + try (ResultSet rs = ps.executeQuery()) { + if (!rs.next()) return Deletion.NOT_FOUND; + if (rs.getBoolean("enabled")) return Deletion.STILL_ON; + } + } + boolean used; + try (PreparedStatement ps = c.prepareStatement("SELECT 1 FROM factory_run WHERE harness_credential_id = ? LIMIT 1")) { + ps.setObject(1, id); + try (ResultSet rs = ps.executeQuery()) { used = rs.next(); } + } + if (used) { + try (PreparedStatement ps = c.prepareStatement(""" + UPDATE harness_credential SET api_key = NULL, account_ref = NULL, erased_at = now(), updated_at = now() + WHERE id = ? + """)) { + ps.setObject(1, id); + ps.executeUpdate(); + } + return Deletion.ERASED; + } + // The sign-in that produced a seat is history; it keeps its row and forgets the member. + try (PreparedStatement ps = c.prepareStatement("UPDATE harness_sign_in SET credential_id = NULL WHERE credential_id = ?")) { + ps.setObject(1, id); + ps.executeUpdate(); + } + try (PreparedStatement ps = c.prepareStatement("DELETE FROM harness_credential WHERE id = ?")) { + ps.setObject(1, id); + ps.executeUpdate(); + } + return Deletion.DELETED; + } + /** Bring a disabled member back, because disabling is not deletion and must not be one-way. */ public boolean enable(UUID id) { try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement( - "UPDATE harness_credential SET enabled = TRUE, updated_at = now() WHERE id = ? AND NOT enabled")) { + "UPDATE harness_credential SET enabled = TRUE, updated_at = now() WHERE id = ? AND NOT enabled AND erased_at IS NULL")) { ps.setObject(1, id); return ps.executeUpdate() == 1; } catch (SQLException e) { diff --git a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java index d8a555bc..da4a5431 100644 --- a/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java +++ b/spire-orchestrator/src/main/java/dev/codespire/orchestrator/factory/HarnessCredentialResource.java @@ -113,6 +113,25 @@ public Response disable(@PathParam("id") String id) { return Response.noContent().build(); } + /** + * Delete a switched-off member: outright when no run used it, otherwise by erasing its secret and hiding + * it, keeping the name a finished run shows. A member still switched on is refused (409): a build could + * be picking it at that moment. + */ + @POST + @Path("/{id}/delete") + @Consumes(MediaType.WILDCARD) + public Response delete(@PathParam("id") String id) { + HarnessCredentialPool.Deletion outcome = pool.delete(uuid(id)); + switch (outcome) { + case NOT_FOUND -> throw new NotFoundException("no such harness credential: " + id); + case STILL_ON -> throw new ClientErrorException(Response.status(Response.Status.CONFLICT) + .entity("harness_credential_still_on").type(MediaType.TEXT_PLAIN).build()); + case DELETED, ERASED -> LOG.warnf("harness credential %s was deleted by an operator (%s)", id, outcome); + } + return Response.ok(java.util.Map.of("outcome", outcome.name())).build(); + } + /** Return a disabled member to the pool. Disabling is not deletion, so it is not one-way. */ @POST @Path("/{id}/enable") diff --git a/spire-orchestrator/src/main/resources/db/migration/V86__harness_credential_delete.sql b/spire-orchestrator/src/main/resources/db/migration/V86__harness_credential_delete.sql new file mode 100644 index 00000000..a3f04849 --- /dev/null +++ b/spire-orchestrator/src/main/resources/db/migration/V86__harness_credential_delete.sql @@ -0,0 +1,19 @@ +-- Deleting a harness key or subscription seat (operator feedback, 2026-09-27). +-- +-- A member no run used is deleted outright. A member a run used cannot be: factory_run points at it, +-- and that attribution is the record of who paid. So such a member is ERASED instead: its stored key or +-- sign-in file is removed, it can never be switched on again, and it disappears from the pool screen. +-- The row stays only so a finished run can still name what paid for it. +ALTER TABLE harness_credential ALTER COLUMN api_key DROP NOT NULL; +ALTER TABLE harness_credential ADD COLUMN erased_at TIMESTAMPTZ; + +-- A live member has a secret; an erased one has none, is off, and names no account. +ALTER TABLE harness_credential ADD CONSTRAINT harness_credential_secret_unless_erased + CHECK (erased_at IS NOT NULL OR api_key IS NOT NULL); +ALTER TABLE harness_credential ADD CONSTRAINT harness_credential_erased_holds_nothing + CHECK (erased_at IS NULL OR (api_key IS NULL AND NOT enabled AND account_ref IS NULL)); + +-- An erased member's name is free again, so the operator can reuse it. It stays on the erased row, which +-- is what a finished run shows. +ALTER TABLE harness_credential DROP CONSTRAINT harness_credential_label_key; +CREATE UNIQUE INDEX harness_credential_label_live ON harness_credential (label) WHERE erased_at IS NULL; diff --git a/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessCredentialDeleteTest.java b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessCredentialDeleteTest.java new file mode 100644 index 00000000..6b74bbb9 --- /dev/null +++ b/spire-orchestrator/src/test/java/dev/codespire/orchestrator/factory/HarnessCredentialDeleteTest.java @@ -0,0 +1,135 @@ +package dev.codespire.orchestrator.factory; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.security.TestSecurity; +import jakarta.inject.Inject; +import org.junit.jupiter.api.Test; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.UUID; + +import static io.restassured.RestAssured.given; +import static org.junit.jupiter.api.Assertions.*; + +/** + * Deleting a harness key or seat (operator feedback, 2026-09-27): switching off kept the key or the + * person's sign-in file stored for ever. Placeholder values throughout. + */ +@QuarkusTest +class HarnessCredentialDeleteTest { + + @Inject HarnessCredentialPool pool; + @Inject FactoryRunProjection runs; + @Inject HarnessSignIns signIns; + @Inject dev.codespire.encryption.EncryptionService encryption; + @Inject DataSource dataSource; + + private static String label() { + return "TEST-delete-" + UUID.randomUUID(); + } + + private UUID switchedOffKey(String label) { + UUID id = pool.add(label, "openai", "https://api.openai.com", "TEST-key-to-delete").id(); + pool.remove(id); + return id; + } + + private long rows(String sql, Object id) throws SQLException { + try (Connection c = dataSource.getConnection(); PreparedStatement ps = c.prepareStatement(sql)) { + ps.setObject(1, id); + try (ResultSet rs = ps.executeQuery()) { rs.next(); return rs.getLong(1); } + } + } + + private boolean listed(UUID id) { + return pool.list().stream().anyMatch(member -> member.id().equals(id)); + } + + /** A member no run used is gone, and its name is free again. */ + @Test + void aMemberNoRunUsedIsDeletedOutright() throws SQLException { + String label = label(); + UUID id = switchedOffKey(label); + + assertEquals(HarnessCredentialPool.Deletion.DELETED, pool.delete(id)); + + assertEquals(0, rows("SELECT count(*) FROM harness_credential WHERE id = ?", id)); + UUID again = pool.add(label, "openai", "https://api.openai.com", "TEST-key-again").id(); + pool.remove(again); + } + + /** + * A member a run used keeps the row the run points at, but loses its secret, leaves the screen and can + * never be switched on again. + */ + @Test + void aMemberARunUsedIsErasedAndKeptOnlyForTheRun() throws SQLException { + String label = label(); + UUID id = switchedOffKey(label); + String runId = "run::github:TEST-acme/app:subject-" + UUID.randomUUID() + ":1"; + assertTrue(runs.queued(new FactoryRunProjection.QueuedRun(runId, "codex", "TEST-model", "main", + "abc1234", "spire/TEST-delete", "TEST-bot", id), null, null)); + + assertEquals(HarnessCredentialPool.Deletion.ERASED, pool.delete(id)); + + assertEquals(1, rows("SELECT count(*) FROM harness_credential WHERE id = ? AND api_key IS NULL AND erased_at IS NOT NULL", id), + "the secret is gone; the row stays for the run's attribution"); + assertFalse(listed(id), "an erased member is not on the pool screen"); + assertFalse(pool.enable(id), "an erased member has nothing to switch on"); + HarnessSignIns.Started signIn = signIns.start(label, "codex", "TEST-operator"); + assertNull(signIn.refusal(), "an erased member's name is free for a new sign-in"); + signIns.cancel(signIn.view().id(), "TEST: only the name was checked"); + UUID again = pool.add(label, "openai", "https://api.openai.com", "TEST-key-again").id(); + pool.remove(again); + } + + /** A member still switched on could be picked by a build at this moment, so it must be switched off first. */ + @Test + void aMemberStillSwitchedOnIsRefused() { + UUID id = pool.add(label(), "openai", "https://api.openai.com", "TEST-key-on").id(); + try { + assertEquals(HarnessCredentialPool.Deletion.STILL_ON, pool.delete(id)); + assertTrue(listed(id)); + } finally { + pool.remove(id); + } + } + + /** A seat's sign-in record stays as history and forgets the seat. */ + @Test + void aSeatsSignInRecordForgetsTheDeletedSeat() throws SQLException { + String label = "TEST-delete-seat-" + UUID.randomUUID(); + HarnessSignIns.Started started = signIns.start(label, "codex", "TEST-operator"); + assertNull(started.refusal(), started.refusal()); + UUID signIn = started.view().id(); + String file = "{\"auth_mode\":\"TEST-chatgpt\",\"tokens\":{\"account_id\":\"TEST-account-" + UUID.randomUUID() + "\"}}"; + signIns.completed(new dev.codespire.contract.event.HarnessSignInResult.Completed(signIn.toString(), + encryption.encryptString(file, dev.codespire.contract.event.HarnessSignInResult.sealedAad(signIn.toString())), + "TEST-chatgpt", "TEST-chatgpt")); + UUID seat = signIns.get(signIn).orElseThrow().credentialId(); + pool.remove(seat); + + assertEquals(HarnessCredentialPool.Deletion.DELETED, pool.delete(seat)); + + assertNull(signIns.get(signIn).orElseThrow().credentialId()); + } + + @Test + @TestSecurity(user = "op", roles = "spire-admin") + void theScreenIsToldToSwitchItOffFirst() { + UUID id = pool.add(label(), "openai", "https://api.openai.com", "TEST-key-rest").id(); + try { + given().when().post("/api/harness-credentials/" + id + "/delete") + .then().statusCode(409).body(org.hamcrest.Matchers.containsString("harness_credential_still_on")); + pool.remove(id); + given().when().post("/api/harness-credentials/" + id + "/delete") + .then().statusCode(200).body("outcome", org.hamcrest.Matchers.is("DELETED")); + } finally { + pool.remove(id); + } + } +} diff --git a/spire-ui/src/api.ts b/spire-ui/src/api.ts index 3e858f54..52ed8a09 100644 --- a/spire-ui/src/api.ts +++ b/spire-ui/src/api.ts @@ -1732,6 +1732,13 @@ export const enableHarnessCredential = (id: string) => credentialAction(id, '/en export const clearHarnessCredentialRejection = (id: string) => credentialAction(id, '/clear-rejection', 'POST', 'Failed to clear the rejection'); export const restHarnessCredential = (id: string) => credentialAction(id, '/rest', 'POST', 'Failed to rest the credential'); +/** DELETED: gone. ERASED: a run used it, so its secret is erased and only its name stays, for that run. */ +export async function deleteHarnessCredential(id: string): Promise<'DELETED' | 'ERASED'> { + const res = await apiFetch(`/api/harness-credentials/${encodeURIComponent(id)}/delete`, { method: 'POST' }); + if (!res.ok) return throwResponse(res, 'Failed to delete the credential'); + return (await res.json()).outcome; +} + /** * A subscription sign-in in progress (M3.5 part F). * diff --git a/spire-ui/src/components/SettingsHarnessCredentials.test.tsx b/spire-ui/src/components/SettingsHarnessCredentials.test.tsx index e6db698e..644f227b 100644 --- a/spire-ui/src/components/SettingsHarnessCredentials.test.tsx +++ b/spire-ui/src/components/SettingsHarnessCredentials.test.tsx @@ -131,3 +131,21 @@ it('says why a second seat of one account cannot be switched back on', async () expect(await screen.findByText('Another seat is already signed in to this account. Switch that one off first.')).toBeInTheDocument(); }); + +// Switching off kept the key or the sign-in stored for ever; Delete erases it (feedback, 2026-09-27). +it('deletes a switched-off member only after the operator confirms, and never one still on', async () => { + const remove = vi.spyOn(api, 'deleteHarnessCredential').mockResolvedValue('ERASED'); + vi.mocked(api.fetchHarnessCredentials).mockResolvedValue([ + member({ id: 'TEST-off', label: 'TEST-off', enabled: false }), + member({ id: 'TEST-on', label: 'TEST-on' }), + ]); + render(); + + expect(within(await row('TEST-on')).queryByRole('button', { name: 'Delete' })).toBeNull(); + fireEvent.click(within(await row('TEST-off')).getByRole('button', { name: 'Delete' })); + expect(remove).not.toHaveBeenCalled(); + fireEvent.click(within(await row('TEST-off')).getByRole('button', { name: 'Delete it' })); + + await waitFor(() => expect(remove).toHaveBeenCalledWith('TEST-off')); + expect(await screen.findByText('TEST-off is deleted. Runs that used it still show its name; its secret is erased.')).toBeInTheDocument(); +}); diff --git a/spire-ui/src/components/SettingsHarnessCredentials.tsx b/spire-ui/src/components/SettingsHarnessCredentials.tsx index fc18ecaf..a13ded74 100644 --- a/spire-ui/src/components/SettingsHarnessCredentials.tsx +++ b/spire-ui/src/components/SettingsHarnessCredentials.tsx @@ -1,7 +1,7 @@ import { useEffect, useState } from 'react'; import { KeyRound } from 'lucide-react'; import { - addHarnessCredential, clearHarnessCredentialRejection, disableHarnessCredential, + addHarnessCredential, clearHarnessCredentialRejection, deleteHarnessCredential, disableHarnessCredential, enableHarnessCredential, fetchHarnessCredentials, restHarnessCredential, type HarnessCredentialView, } from '../api'; @@ -33,6 +33,7 @@ const when = (value: string | null) => (value ? new Date(value).toLocaleString() function sentence(message: string): string { if (message.includes('subscription_account_taken')) return 'Another seat is already signed in to this account. Switch that one off first.'; + if (message.includes('harness_credential_still_on')) return 'Switch it off before deleting it.'; return message; } @@ -51,6 +52,8 @@ export default function SettingsHarnessCredentials() { const [error, setError] = useState(''), [notice, setNotice] = useState(''); const [adding, setAdding] = useState(false), [busy, setBusy] = useState(false); const [signingIn, setSigningIn] = useState(false); + // The member an operator asked to delete, waiting for them to confirm. + const [deleting, setDeleting] = useState(null); const [form, setForm] = useState({ label: '', type: 'openai', baseUrl: '', apiKey: '' }); const [refresh, setRefresh] = useState(0); @@ -68,7 +71,19 @@ export default function SettingsHarnessCredentials() { setBusy(true); setError(''); try { await action(); reload(message); } catch (failure) { setError(sentence(String(failure instanceof Error ? failure.message : failure))); } - finally { setBusy(false); } + finally { setBusy(false); setDeleting(null); } + } + + // Its own path rather than act(): the message depends on what the server did. + async function remove(member: HarnessCredentialView) { + setBusy(true); setError(''); + try { + const outcome = await deleteHarnessCredential(member.id); + reload(outcome === 'ERASED' + ? `${member.label} is deleted. Runs that used it still show its name; its secret is erased.` + : `${member.label} is deleted.`); + } catch (failure) { setError(sentence(String(failure instanceof Error ? failure.message : failure))); } + finally { setBusy(false); setDeleting(null); } } async function save() { @@ -174,10 +189,20 @@ export default function SettingsHarnessCredentials() { Switch off ) : ( + <> + {deleting !== member.id && } + {deleting === member.id && <> + Its stored key or sign-in is erased for good. + + + } + )} diff --git a/spire-ui/src/components/accounts.ts b/spire-ui/src/components/accounts.ts index 9850fa9e..782bdcd0 100644 --- a/spire-ui/src/components/accounts.ts +++ b/spire-ui/src/components/accounts.ts @@ -45,8 +45,10 @@ export function scopeLabel(scopes: string | null | undefined): string { * FACTORY and a migrated CONTEXT row. The kind and role are what tell them apart. */ export function accountOptionLabel( - account: { name: string; type: string; role: string; enabled: boolean }, + account: { name: string; type: string; role: string; enabled: boolean; botUsername?: string | null }, ): string { + // The bot handle too: two accounts can share a name, and the handle is what a ticket comment shows. return `${account.name} · ${account.type} · ${roleLabel(account.role)}` + + (account.botUsername ? ` · @${account.botUsername}` : '') + (account.enabled ? '' : ' (disabled)'); } diff --git a/spire-ui/src/components/repositories/factory/BuildStep.tsx b/spire-ui/src/components/repositories/factory/BuildStep.tsx index bb26e417..be6377eb 100644 --- a/spire-ui/src/components/repositories/factory/BuildStep.tsx +++ b/spire-ui/src/components/repositories/factory/BuildStep.tsx @@ -82,6 +82,9 @@ export default function BuildStep({ repositoryId, defaults, open, setOpen, chang const levels = known?.status === 'OK' ? known.offered.find(model => model.slug === form.model)?.efforts ?? [] : []; const levelUnusable = !!form.effort && !levels.includes(form.effort); const complete = !!form.baseBranch.trim() && !!form.harness && !!picked && picked.blocked === null && !levelUnusable; + // Save stays off until these are filled; saying which is what an operator needs (feedback, 2026-09-27). + const missing = [!form.baseBranch.trim() && 'Base branch', !form.harness && 'Harness', form.harness && !form.model && 'Model'] + .filter((field): field is string => !!field); return 0 ? 'done' : 'missing'} actions={!editing && }> @@ -129,6 +132,7 @@ export default function BuildStep({ repositoryId, defaults, open, setOpen, chang {error.includes('Reload it') &&

} {busy === 'saving' &&

Saving the build setup. The form unlocks when the server answers.

} + {busy === null && missing.length > 0 &&

To save, fill in: {missing.join(', ')}.

}
diff --git a/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx b/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx index b7cf3c21..8cca457f 100644 --- a/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx +++ b/spire-ui/src/components/repositories/factory/RepositoryFactory.test.tsx @@ -263,6 +263,17 @@ describe('build setup', () => { model: 'TEST-unpriced-only', payWith: 'SUBSCRIPTION' }))); }); + // Save stays off until the form is complete; it now says which field is missing (feedback, 2026-09-27). + it('names the fields that keep the build setup from saving', async () => { + renderFactory(); + await open(); + fireEvent.change(await screen.findByLabelText('Harness', field), { target: { value: 'codex' } }); + + expect(await screen.findByText('To save, fill in: Base branch, Model.')).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText('Base branch', field), { target: { value: 'main' } }); + expect(await screen.findByText('To save, fill in: Model.')).toBeInTheDocument(); + }); + // Every option disabled: the select ignores clicks and keys, which the operator read as broken. it('says why no model can be picked when every one lacks a price', async () => { const types = ['INPUT', 'CACHED_INPUT', 'CACHE_WRITE', 'OUTPUT', 'REASONING']; diff --git a/spire-ui/src/components/repositories/factory/SourceStep.test.tsx b/spire-ui/src/components/repositories/factory/SourceStep.test.tsx index d3dfa7db..b3f6c7eb 100644 --- a/spire-ui/src/components/repositories/factory/SourceStep.test.tsx +++ b/spire-ui/src/components/repositories/factory/SourceStep.test.tsx @@ -54,12 +54,14 @@ it('explains a missing account and links to Accounts instead of an empty picker' expect(within(form).getByRole('link', { name: 'Add an account' })).toHaveAttribute('href', '#/settings/accounts'); expect(within(form).getByRole('button', { name: 'Register work source' })).toBeDisabled(); }); +// The handle too: it is what a ticket comment shows, and two accounts can share a name (feedback, 2026-09-27). it('distinguishes duplicate credential names in the create and edit account pickers', async () => { - const accounts = [account('github', { name: 'TEST-shared name' }), account('github', { id: 'TEST-reviewer', name: 'TEST-shared name', role: 'REVIEWER' })]; + const accounts = [account('github', { name: 'TEST-shared name', botUsername: 'TEST-factory-bot' }), + account('github', { id: 'TEST-reviewer', name: 'TEST-shared name', role: 'REVIEWER', botUsername: 'TEST-reviewer-bot' })]; renderFactory({ accounts }); const expectDistinct = (select: HTMLElement) => { - expect(within(select).getByRole('option', { name: 'TEST-shared name · github · Factory' })).toHaveValue('TEST-github'); - expect(within(select).getByRole('option', { name: 'TEST-shared name · github · Reviewer' })).toHaveValue('TEST-reviewer'); + expect(within(select).getByRole('option', { name: 'TEST-shared name · github · Factory · @TEST-factory-bot' })).toHaveValue('TEST-github'); + expect(within(select).getByRole('option', { name: 'TEST-shared name · github · Reviewer · @TEST-reviewer-bot' })).toHaveValue('TEST-reviewer'); }; fireEvent.click(within(await step(1)).getByRole('button', { name: 'Add another source' })); expectDistinct(within(screen.getByRole('group', { name: 'Add where tickets come from' })).getByLabelText('Tracker account', field)); @@ -165,3 +167,10 @@ it('reports a scan request while it is in flight and says when the scanner reads await act(async () => { release(); }); expect(await screen.findByText(/within about 30 seconds/)).toBeInTheDocument(); }); + +// This account also writes the comments on the tickets, so its role and handle are shown, not only a name +// two accounts can share (feedback, 2026-09-27). +it('names the account that reads the tickets by its role and handle', async () => { + renderFactory(); + expect(within(await step(1)).getByText(/· Factory · @TEST-bot/)).toBeInTheDocument(); +}); diff --git a/spire-ui/src/components/repositories/factory/SourceStep.tsx b/spire-ui/src/components/repositories/factory/SourceStep.tsx index b0dbcf99..70fbfe07 100644 --- a/spire-ui/src/components/repositories/factory/SourceStep.tsx +++ b/spire-ui/src/components/repositories/factory/SourceStep.tsx @@ -1,6 +1,7 @@ import { useState } from 'react'; import type { ProviderView, WebhookRepoView } from '../../../api'; import type { Repository } from '../repositoriesApi'; +import { accountOptionLabel } from '../../accounts'; import * as api from '../../work-items/workSourcesApi'; import FactoryStep from './FactoryStep'; import InstantUpdates from './InstantUpdates'; @@ -22,7 +23,11 @@ export default function SourceStep({ repository, sources, accounts, webhooks, op const [scanning, setScanning] = useState(null); const reading = sources.some(source => source.enabled); const adding = open === 'source:new'; - const accountName = (id: string) => accounts.find(account => account.id === id)?.name ?? 'an unavailable account'; + // Role and handle too: two accounts can share a name, and this one writes the comments on the tickets. + const accountName = (id: string) => { + const account = accounts.find(candidate => candidate.id === id); + return account ? accountOptionLabel(account) : 'an unavailable account'; + }; // The request only sets a flag; the scanner picks it up on its next sweep, about half a minute // later. So the button reports the request, and never claims the scan itself has finished. async function rescan(source: api.WorkSource) { diff --git a/spire-ui/src/components/work-items/DecisionEvidence.tsx b/spire-ui/src/components/work-items/DecisionEvidence.tsx index b39d23c4..9bffcd9e 100644 --- a/spire-ui/src/components/work-items/DecisionEvidence.tsx +++ b/spire-ui/src/components/work-items/DecisionEvidence.tsx @@ -58,9 +58,14 @@ export default function DecisionEvidence({ item, approval, evidence, evidenceErr
Starts from
{preparation.baseBranch} @ {preparation.baseCommit.slice(0, 7)}
Agent
{preparation.harness} · {preparation.model}
Plan
· one step
+
Pays with
{preparation.payWith === 'SUBSCRIPTION' ? 'a Codex subscription' : 'an API key'}
- {evidence?.specification &&
{evidence.specification}
} - {evidence?.instruction &&
{evidence.instruction}
} + {evidence?.specification && <> +

Specification — the ticket text, as it was prepared

+
{evidence.specification}
} + {evidence?.instruction && <> +

Plan — the one step the build runs

+
{evidence.instruction}
} }

If you approve

diff --git a/spire-ui/src/components/work-items/DecisionPanel.test.tsx b/spire-ui/src/components/work-items/DecisionPanel.test.tsx index 8aeaca1d..26140402 100644 --- a/spire-ui/src/components/work-items/DecisionPanel.test.tsx +++ b/spire-ui/src/components/work-items/DecisionPanel.test.tsx @@ -38,6 +38,15 @@ async function approvable() { } function show() { return render(); } +// How it pays is part of what the gate binds, and which text is which was not said (feedback, 2026-09-27). +it('shows how the build pays and labels the specification and the plan', async () => { + vi.mocked(gateway.getWorkItem).mockResolvedValue({ ...item, preparation: { ...item.preparation, payWith: 'SUBSCRIPTION' } }); + show(); + expect(await screen.findByText('Specification — the ticket text, as it was prepared')).toBeInTheDocument(); + expect(screen.getByText('Plan — the one step the build runs')).toBeInTheDocument(); + expect(screen.getByText('a Codex subscription')).toBeInTheDocument(); +}); + // The old card showed a digest and a generation number. An approver has to see what they approve. it('shows what the gate binds before offering an answer', async () => { show(); diff --git a/spire-ui/src/components/work-items/WorkItemDetail.tsx b/spire-ui/src/components/work-items/WorkItemDetail.tsx index 697c71f9..4c324c24 100644 --- a/spire-ui/src/components/work-items/WorkItemDetail.tsx +++ b/spire-ui/src/components/work-items/WorkItemDetail.tsx @@ -17,6 +17,9 @@ import { WorkflowStatus } from './WorkItems'; * One work item as its journey. The heading names the ticket, the steps say where it is and what a * person can do there, and everything else — policy, ticket text, history — folds underneath. */ +/** How often an open work item is read again. */ +const DETAIL_POLL_MILLISECONDS = 10_000; + export default function WorkItemDetail() { const { id = '' } = useParams(); const [params, setParams] = useSearchParams(); @@ -47,6 +50,18 @@ export default function WorkItemDetail() { return () => { active = false; }; }, [id, refresh]); + // Follows the item like Runs does, so a gate opening or a build ending shows without a click. Not while + // a panel is open: a re-read under a decision the operator is reading would move what they approve. + useEffect(() => { + if (panel) return; + const timer = setInterval(() => { + if (document.visibilityState !== 'visible') return; + getWorkItem(id).then(item => setState(previous => previous.item?.id === id ? { item, error: null } : previous)) + .catch(() => { /* the last good read stays on screen; the next tick tries again */ }); + }, DETAIL_POLL_MILLISECONDS); + return () => clearInterval(timer); + }, [id, panel]); + function reread(message = '') { setNotice(message); setRefresh(value => value + 1); } function start() { setNotice(''); return ++action.current; } function open(name: 'decide' | 'prepare') { setParams({ [name]: '1' }); } diff --git a/spire-ui/src/components/work-items/WorkItems.test.tsx b/spire-ui/src/components/work-items/WorkItems.test.tsx index 311e9443..dc5fb9bf 100644 --- a/spire-ui/src/components/work-items/WorkItems.test.tsx +++ b/spire-ui/src/components/work-items/WorkItems.test.tsx @@ -678,3 +678,22 @@ it('offers composing again while a plan decision is open', async () => { fireEvent.click(await screen.findByRole('button', { name: 'Prepare again from the ticket' })); await waitFor(() => expect(compose).toHaveBeenCalledWith(detail().id, detail().revision)); }); + +// The page follows the item like Runs does, so a build ending shows without a click (feedback, 2026-09-27). +it('reads the open work item again on its own', async () => { + vi.spyOn(auth, 'fetchMe').mockResolvedValue({ authEnabled: true, authenticated: true, user: 'TEST-admin', roles: ['spire-admin'] }); + vi.spyOn(api, 'getWorkItem').mockResolvedValue(detail()); + vi.spyOn(api, 'getWorkItemTracker').mockResolvedValue({ title: 'TEST-title', body: 'TEST-body', trackerStatus: 'open' }); + vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('visible'); + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + showDetail(); + await waitFor(() => expect(api.getWorkItem).toHaveBeenCalledTimes(1)); + + await act(async () => { vi.advanceTimersByTime(10_000); }); + + await waitFor(() => expect(api.getWorkItem).toHaveBeenCalledTimes(2)); + } finally { + vi.useRealTimers(); + } +}); diff --git a/spire-ui/src/components/work-items/WorkItems.tsx b/spire-ui/src/components/work-items/WorkItems.tsx index da278612..7e70b2a3 100644 --- a/spire-ui/src/components/work-items/WorkItems.tsx +++ b/spire-ui/src/components/work-items/WorkItems.tsx @@ -96,6 +96,8 @@ export default function WorkItems() { {notice &&

{notice}

} +

A labelled ticket is found by a scan that runs on a schedule (every 30 seconds by + default), then prepared. It can take a minute or two to appear here. This list refreshes by itself.

{needs !== null && needs > 0 &&