fix(admin): a run blocked on an unreachable backend must not render "ok"
Ed pointed a broken S3 entry at 127.0.0.1 with nothing listening. The classification worked end to end — the run paused with `target backend init: Transient Backend: … ConnectionRefused` — but the job row showed a green **ok** pill and the reason was only visible after expanding it. `PausedRetryable` reports `outcome: 'ok'` on the wire, and that is correct: the run did not fail, and a Resume continues it. But rendering it as plain "ok" hides the one thing worth acting on. A paused `backend_migration` is still holding `migration_readonly` and refusing writes across the whole application, presented as a healthy job. The row now reads **blocked**, in amber, with the reason as the pill's title so it is legible without unfolding anything. Amber rather than red, deliberately: nothing is broken and no data was lost — the run is waiting for the backend to return. Red reads as "this job is failing" and invites a Cancel, which for a migration also discards the copy already done and is the one action that cannot be undone. Checked BEFORE the findings branches, too. A run that never finished has nothing meaningful to say about findings, and "0 issues" on an aborted scan is a worse answer than "blocked". `JobOutcome.extra` was typed `unknown`, so the panel could not read the `retryable` flag the backend already sends. Now a narrow `JobOutcomeExtra` exposing just the three keys that describe the RUN's shape rather than its work — the rest stay per-job counters nothing generic should switch on. Same class of defect as the known "Ok despite findings" issue: an outcome that looks like success while hiding the state an operator needs to see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -597,9 +597,33 @@ export interface FolderAncestorsResponse {
|
||||
* discriminant is the `outcome` field, not the object key.
|
||||
*/
|
||||
export type JobOutcome =
|
||||
| { outcome: 'ok'; count: number; extra?: unknown }
|
||||
| { outcome: 'ok'; count: number; extra?: JobOutcomeExtra }
|
||||
| { outcome: 'err'; message: string };
|
||||
|
||||
/**
|
||||
* The parts of a job outcome's free-form `extra` the panel reads.
|
||||
*
|
||||
* Deliberately narrow — most keys are per-job counters nothing generic
|
||||
* should switch on. These three describe the RUN's shape rather than
|
||||
* its work, and the panel has to render them:
|
||||
*
|
||||
* A run that stopped because the backend was unreachable reports
|
||||
* `outcome: 'ok'` — it did not fail, it paused and can be resumed. Read
|
||||
* alone that renders as a green "ok" pill, which is exactly wrong: a
|
||||
* paused `backend_migration` still holds `migration_readonly` and is
|
||||
* refusing writes application-wide. `retryable` is what lets the row
|
||||
* say so.
|
||||
*/
|
||||
export interface JobOutcomeExtra {
|
||||
/** The run stopped at its cursor and can be resumed. */
|
||||
paused?: boolean;
|
||||
/** It stopped because the ENVIRONMENT failed, not because an
|
||||
* operator asked — `reason` says what. */
|
||||
retryable?: boolean;
|
||||
reason?: string;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
/**
|
||||
* `JobSummary` — one row per registered job in `GET /api/admin/jobs`.
|
||||
* Cadence + last-run bookkeeping. `interval_ms` / `next_run_at` are
|
||||
|
||||
@@ -516,8 +516,32 @@
|
||||
}
|
||||
}
|
||||
|
||||
// A run that stopped because the backend was unreachable, rather than
|
||||
// because an operator asked it to stop.
|
||||
//
|
||||
// It reports `outcome: 'ok'` on the wire — correctly, since it did
|
||||
// not fail and a Resume continues it — but rendering that as a plain
|
||||
// green "ok" hides the thing worth acting on. A paused
|
||||
// `backend_migration` is still holding `migration_readonly` and
|
||||
// refusing writes across the whole app; the row has to say so.
|
||||
function stoppedOnBackendFailure(job: JobSummary): boolean {
|
||||
return job.last_outcome?.outcome === 'ok' && job.last_outcome.extra?.retryable === true;
|
||||
}
|
||||
|
||||
function backendFailureReason(job: JobSummary): string | undefined {
|
||||
if (job.last_outcome?.outcome !== 'ok') return undefined;
|
||||
const reason = job.last_outcome.extra?.reason;
|
||||
return typeof reason === 'string' ? reason : undefined;
|
||||
}
|
||||
|
||||
function outcomeLabel(job: JobSummary): string {
|
||||
if (!job.last_outcome) return t('admin.jobs.never', 'never');
|
||||
// Checked before the findings branches: a run that never finished
|
||||
// has nothing meaningful to say about findings, and "0 issues" on
|
||||
// an aborted scan is a worse answer than "blocked".
|
||||
if (stoppedOnBackendFailure(job)) {
|
||||
return t('admin.jobs.outcome_blocked', 'blocked');
|
||||
}
|
||||
if (job.last_outcome.outcome === 'ok') {
|
||||
// `ok` on the wire = dispatch completed. If any actionable
|
||||
// findings surfaced, we flip to "issues" (amber). If only
|
||||
@@ -539,6 +563,14 @@
|
||||
if (job.last_outcome.outcome !== 'ok') {
|
||||
return 'jobs-panel__pill jobs-panel__pill--err';
|
||||
}
|
||||
// Amber, not red: nothing is broken and no data was lost — the
|
||||
// run is waiting for the backend to come back and a Resume
|
||||
// continues it. Red would read as "this job is failing" and
|
||||
// invite a cancel, which for a migration also throws away the
|
||||
// copy already done.
|
||||
if (stoppedOnBackendFailure(job)) {
|
||||
return 'jobs-panel__pill jobs-panel__pill--paused';
|
||||
}
|
||||
if (actionableFindingCount(job) > 0) {
|
||||
return 'jobs-panel__pill jobs-panel__pill--paused';
|
||||
}
|
||||
@@ -912,7 +944,14 @@
|
||||
<td class="jobs-panel__muted">{timeAgo(job.last_run_at)}</td>
|
||||
<td>
|
||||
<div class="jobs-panel__outcome-cell">
|
||||
<span class={outcomeClass(job)}>{outcomeLabel(job)}</span>
|
||||
<!-- The reason is on the pill itself, not only in the
|
||||
expanded drawer: it is the whole content of a
|
||||
"blocked" row, and folding it away is what made a
|
||||
migration paused by an unreachable endpoint read as
|
||||
a plain green "ok". -->
|
||||
<span class={outcomeClass(job)} title={backendFailureReason(job)}
|
||||
>{outcomeLabel(job)}</span
|
||||
>
|
||||
{#if actionableFindingCount(job) > 0}
|
||||
{@const findings = actionableFindingCount(job)}
|
||||
<span
|
||||
|
||||
Reference in New Issue
Block a user