From 602d5d9583fe69258fbbed3ae9c1bb4c97c7b903 Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Thu, 16 Jul 2026 09:20:17 +0200 Subject: [PATCH] =?UTF-8?q?fix(cockpit):=20derive=20accurate=20live=20task?= =?UTF-8?q?=20state=20=E2=80=94=20no=20more=20phantom=20in-flight=20worker?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live-progress panel keyed worker identity on the RAW task string, so the same worker logging "81" then "issue-81" (or "issue-70-worker-inspector") became several identities, and a "done" logged under one variant never overwrote the "reviewing" logged under another. Combined with workers that crash or simply never emit their own done, the cockpit showed tasks 70/81 as reviewing/implementing a week after their PRs merged. Three derivation rules fix it: - Worker identity is (role, normalized task, lens): id variants collapse via the existing taskGroupKey() normalizer, so a later done actually overwrites the earlier phase. Lens stays in the key — reviewers of the same task under different lenses are distinct workers. - The orchestrator owns the task lifecycle: a group whose orchestrator's latest event is done — with no worker activity logged after it (file order) — renders as a single done-badged header, not a table of phantom in-flight rows. No orchestrator events falls back to every-worker-done. Activity logged AFTER an orchestrator done (task re-scoped) keeps the group active. - Unfinished groups silent for >COCKPIT_STALE_AFTER_SECONDS (default 2h, same override style as COCKPIT_NOW) are badged "stale · no events for Nh/Nd" with muted rows — a wedged worker should be visible as wedged, never as active work. Verified against the real event log: 70/81 (and 8 other finished tasks) now render done, three genuinely-abandoned groups render stale, and the one task the loop is actually driving (#95, scoped 25 min ago) still renders active. Adds 10 checks to cockpit.test.sh (fixture mirrors the exact production log shape that exposed the bug); all 80 existing checks pass unchanged. Co-Authored-By: Claude Fable 5 --- .claude/scripts/cockpit.sh | 67 ++++++++++++++++++++- .claude/scripts/cockpit.test.sh | 101 ++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 3 deletions(-) diff --git a/.claude/scripts/cockpit.sh b/.claude/scripts/cockpit.sh index b246f35..e8ff9e5 100755 --- a/.claude/scripts/cockpit.sh +++ b/.claude/scripts/cockpit.sh @@ -15,6 +15,13 @@ # first), and a STALLED banner if no tick has landed in over 2x the cadence's # expected interval (FAST=60s -> 120s, WATCH=300s -> 600s, IDLE=900s -> 1800s). # +# The live panel derives ACCURATE task state, not just last-event-per-raw-key +# (the stale-cockpit fix): worker identity is (role, normalized task, lens) so +# id variants like "81"/"issue-81" merge; a task whose orchestrator logged +# "done" last is rendered as one done-badged header (no phantom in-flight +# rows); an unfinished task silent for >COCKPIT_STALE_AFTER_SECONDS (default +# 2h) is badged "stale" with muted rows instead of reading as active work. +# # Usage: # cockpit.sh [--fixtures ] [output-path] # cockpit.sh --parse-blocking @@ -424,11 +431,21 @@ function prBadge(pr) { } function renderLiveProgress() { - const latest = new Map(); // "role\u0000task" -> event + // Worker identity is (role, NORMALIZED task, lens) - not the raw task + // string. Workers log the same task inconsistently ("81", "issue-81", + // "issue-70-worker-inspector"), and keying on the raw string meant a + // "done" logged under one variant never overwrote the "reviewing" logged + // under another, leaving phantom in-flight rows forever (the stale-cockpit + // bug on issues 70/81). Lens stays in the key: two reviewers of the same + // task under different lenses are genuinely distinct workers. + const latest = new Map(); // "role\u0000groupKey\u0000lens" -> event + let seq = 0; for (const ev of events) { const role = ev.role != null ? String(ev.role) : ""; const task = ev.task != null ? String(ev.task) : ""; - const key = role + "\u0000" + task; + const lens = ev.lens != null ? String(ev.lens) : ""; + const key = role + "\u0000" + taskGroupKey(task).key + "\u0000" + lens; + ev._seq = seq++; // file order == append order; used by the finished check latest.set(key, ev); // later lines overwrite earlier ones for the same key } const workers = [...latest.values()]; @@ -475,18 +492,52 @@ function renderLiveProgress() { const pr = findPRForIssue(g.num); if (pr) header += ` · PR ${prBadge(pr)}`; } + // Task-level terminal state (stale-cockpit fix): the orchestrator owns + // the task lifecycle, so a group whose orchestrator's latest event is + // "done" — with no worker activity logged AFTER it (_seq = file order) + // — is finished, even when a sub-worker never logged its own "done" + // (crashed, or logged it under a task-id variant the old raw-string + // keying missed). No orchestrator events at all falls back to "every + // worker done". Finished groups render as one done-badged header row, + // not a table of phantom "implementing"/"reviewing" workers. + const orch = g.workers.filter((w) => String(w.role) === "orchestrator"); + const orchDoneSeq = orch.length > 0 && orch.every((w) => w.phase === "done") + ? Math.max(...orch.map((w) => w._seq || 0)) : -1; + const lastActiveSeq = g.workers.reduce( + (acc, w) => (w.phase !== "done" && (w._seq || 0) > acc ? (w._seq || 0) : acc), -1); + const finished = orchDoneSeq >= 0 + ? orchDoneSeq > lastActiveSeq + : g.workers.every((w) => w.phase === "done"); + // Staleness (same fix): an unfinished group with no events for over + // STALE_AFTER_SECONDS is far more likely a crashed/wedged worker than + // live work — badge it and mute its rows so it never reads as active. + const newestMs = g.workers.reduce((acc, w) => { + const t = Date.parse(String(w.ts || "")); + return Number.isFinite(t) && t > acc ? t : acc; + }, -Infinity); + const stale = !finished && Number.isFinite(newestMs) + && nowMs - newestMs > STALE_AFTER_SECONDS * 1000; + if (finished) header += ` done`; + if (stale) { + const hours = Math.floor((nowMs - newestMs) / 3600000); + const age = hours >= 48 ? `${Math.floor(hours / 24)}d` : `${hours}h`; + header += ` stale · no events for ${esc(age)}`; + } // Group-header row: a full-width so it never collides // with the "" pattern a plain worker row starts with (tests // and the client sort script both rely on being able to tell the two // apart) — it uses instead of a bare . html += `${header}`; + if (finished) continue; const rows = g.workers.slice().sort((a, b) => { const ar = String(a.role || ""), br = String(b.role || ""); if (ar !== br) return ar.localeCompare(br); return String(a.task || "").localeCompare(String(b.task || "")); }); for (const w of rows) { - const badge = phaseBadge(w.phase); + // Stale groups mute every phase badge: a week-old "implementing" + // rendered warn-yellow is exactly the lie this fix removes. + const badge = stale ? { cls: "muted" } : phaseBadge(w.phase); html += `${esc(w.role)}${esc(w.task)}${esc(w.model || "(none)")}`; html += `${esc(w.phase || "(unknown)")}`; html += `${esc(w.lens || "")}${esc(w.ts)}`; @@ -521,6 +572,16 @@ const VERDICT_HISTORY_N = (() => { const n = parseInt(process.env.COCKPIT_VERDICT_HISTORY_N, 10); return Number.isFinite(n) && n > 0 ? n : 10; })(); +// Live-progress staleness threshold (stale-cockpit fix, see +// renderLiveProgress): an unfinished task group with no events for longer +// than this is badged "stale" instead of rendering as active work. Default +// 2h — long enough for a slow gate run, far shorter than the days-old +// phantom workers this guards against. Same override style as the consts +// above; COCKPIT_NOW pins "now" for tests. +const STALE_AFTER_SECONDS = (() => { + const n = parseInt(process.env.COCKPIT_STALE_AFTER_SECONDS, 10); + return Number.isFinite(n) && n > 0 ? n : 7200; +})(); function renderLoopHealth() { let html = `

Loop health

`; if (ticks.length === 0) { diff --git a/.claude/scripts/cockpit.test.sh b/.claude/scripts/cockpit.test.sh index b8d9c87..bd66df8 100755 --- a/.claude/scripts/cockpit.test.sh +++ b/.claude/scripts/cockpit.test.sh @@ -821,6 +821,107 @@ check "live-progress table headers carry data-sort-key attributes for the client check "output HTML has no external