diff --git a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_brief.md b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_brief.md index 55efc386040..eb35e86c816 100644 --- a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_brief.md +++ b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_brief.md @@ -51,7 +51,15 @@ which stay local and are never rendered into a comment. your open-item limit. 5. **Write the ledger before ending the turn. Always** — including turns where nothing moved, because "checked at 20:44, still waiting on CI round 3" is the - difference between a working crew and a crew that looks asleep. + difference between a working crew and a crew that looks asleep. When you took + nothing at all — the queue held nothing in your scope, or every candidate was + already claimed or already skipped — record that too, as a crew-level step: + call the record tool with **no** `number` and `event_kind: "sweep"`. It is the + one write that belongs to no issue, so it is the only way an empty cycle + becomes visible instead of looking like a crew that stopped running. + Consecutive sweeps coalesce, so calling it on every idle turn does not grow + the log — the answer tells you whether your report was appended or folded into + the open stretch. Also write the ledger at any natural checkpoint inside a turn — before a long build, before a push, before anything that might hit the 2-hour ceiling. diff --git a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_ledger_spec.md b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_ledger_spec.md index 60901027027..93eb97d9a44 100644 --- a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_ledger_spec.md +++ b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_ledger_spec.md @@ -276,8 +276,46 @@ a lock and was caught in review — take its `_LedgerLock` too). "crew_id":"c_7f3a","number":2251,"kind":"ci","text":"CI round 3 — 41/47 green, 6 inherited from main"} ``` +`number` is OMITTED on a **crew-level** line — a step that belongs to no issue, +which today is only `kind: "sweep"` (the crew checked the queue and took +nothing). The key is absent rather than `null` or `0`: a reader tells "this line +is about no issue" from its absence, and a `0` would be indistinguishable from a +real issue number in every filter and join that keys on it. In the id, an absent +number renders as the empty string, so the formula for a numbered line is +unchanged and the two families cannot collide. + +The pairing is enforced in both directions, in the write route and in the store: +a numberless line must carry a crew-level kind, and a crew-level kind must not +carry a number. Neither shape can drift into the other's meaning. + +**Consecutive sweeps coalesce.** "Checked, took nothing" is a recurring +latest-value fact, not an event, and a crew is nudged on a timer — so one line per +idle cycle would be an unbounded run. Reads here are capped and discard the OLDEST +line first, so a crew idling a couple of hundred cycles would push its real work +history out of its own work log. The FIRST sweep after real work is written; a +sweep whose crew already has one as its newest line is answered with that existing +line, and the response says `coalesced`. The surviving timestamp therefore marks +when the idle stretch BEGAN, which is the more useful reading. This is the same +record-the-transition discipline `phase` already follows — stamped only when an +item is created or actually moves. The tail check and the append happen under ONE +hold of the events lock, or two crew turns waking together would both see no +trailing sweep and both append. + +**What the surviving timestamp does not mean.** It records when the crew last +REPORTED an empty queue, and nothing about the present. Consecutive reports fold, +and a crew that stops — an operator pause, a crash, a lost nudge timer — stops +reporting without saying so, because nothing on the crew record evidences liveness. +So the ledger cannot tell a crew that is still checking from one that quietly died +mid-stretch, and the crew page renders the line as the past instant it is rather +than as a claim about now. An earlier revision qualified it as "checking since …", +which read as present-tense activity and so masked exactly the failure an operator +opens that page to notice. Closing that properly needs a real last-seen datum, and +that is a separate change. + One log feeds **two** surfaces: the work-log table on the crew page, and the -`
` progress list inside the public claim comment. +`
` progress list inside the public claim comment. A crew-level line +reaches only the first — it has no issue, so there is no claim comment to render +it into. That dual use imposes the stricter constraint on both: **`text` becomes public**, so it must never contain an absolute path, a host name, or anything from the @@ -396,7 +434,7 @@ five findings fields the same way). Empty fields are dropped, so a partial patch preserves what an earlier write stored. ``` -number required int +number optional int — omit ONLY with `event_kind: sweep` phase optional enum outcome optional enum next optional str @@ -414,9 +452,18 @@ skip_scope optional enum — why a pass was recorded, including `needs-decision` / `needs-investigation` when the next step belongs to a human event optional str — the public progress line -event_kind optional enum (claim|investigate|reply|implement|ci|review|conflict|merge|handback|skip|yield) +event_kind optional enum (claim|investigate|reply|implement|ci|review|conflict|merge|handback|skip|yield|sweep) ``` +Nothing is unconditionally required. `number` was, which left a crew that swept +an empty queue no way to record the cycle without inventing an issue number. The +coupling that replaced the requirement is a relation between two fields, which a +per-field schema cannot express, so it lives on the write route and in the store: +a missing `number` is valid ONLY with `sweep`, `sweep` is invalid WITH one, and a +numberless call takes none of the work-item fields (they patch an item this call +does not create, so they are refused rather than dropped). A present-but-invalid +number stays a 400 and is never reinterpreted as "no issue". + Validation lives in `validation.py` alongside the existing schemas. The handler sends `owner`/`repo` explicitly so a same-numbered issue in another repo cannot overwrite this record, and refuses a second item entering an editing phase. diff --git a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_routes.py b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_routes.py index b3459c971c9..71124423e21 100644 --- a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_routes.py +++ b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_routes.py @@ -828,14 +828,22 @@ async def _revoke_execution( async def _handle_crew_work(request: web.Request) -> web.Response: - """PUT /crew/work {"owner","repo","crew_id","number", ...patch, "event", - "event_kind"} -> {"item","event"}. + """PUT /crew/work {"owner","repo","crew_id","number"?, ...patch, "event", + "event_kind"} -> {"item","event","skip"}. THE route a crew writes its progress through, and the one the MCP write tool targets. It upserts the work item AND appends one ledger line in a single call, which is the whole point: a phase cannot change without a logged reason, because there is no route that changes one without the other. + ``number`` IS OPTIONAL, and omitting it is how a crew records a step that + belongs to no issue — today only a queue sweep that took nothing. That case + exists because the alternative was worse: a crew with an empty queue had no + way to report the cycle except by attributing it to an issue it never touched, + so the record either lied or was not written at all. A numberless write takes + the crew-level path below: one ledger line, no work item, no skip entry, and + no work-item fields accepted. + ORDER MATTERS and is: validate the log line -> write the item -> index a skip -> append the line. Validating the kind and text FIRST means the store's own refusals, including the second-editing-item 409, all happen before anything is @@ -887,13 +895,12 @@ async def _handle_crew_work(request: web.Request) -> web.Response: return early crew_id = routes._str_field(body, "crew_id") - # Reusing the PR field parser rather than copying its bound: the bound is the - # point (the number becomes a FILENAME), and a second copy is how one of them - # ships without it. Its out-of-range text says "pull-request number", which is - # cosmetically wrong here and only reachable past 1e9. - number, number_error = routes._pr_number_field(body) - if number_error is not None: - return number_error + # `number` is OPTIONAL, and its ABSENCE is what selects a crew-level line: a + # step that belongs to no issue, which today is only "swept the queue and took + # nothing". Presence, not truthiness -- a present-but-invalid number is still a + # 400 below rather than being silently reinterpreted as "no issue", because a + # crew that meant an issue must not have a typo read as a queue sweep. + crew_level = "number" not in body event_text, too_long = routes._pr_body_field(body, "event") if too_long is not None: @@ -911,11 +918,70 @@ async def _handle_crew_work(request: web.Request) -> web.Response: "code": "invalid_event_kind"}, status=400, ) + # The vocabulary and the shape are paired in BOTH directions, so neither can + # drift into the other's meaning. A missing number with an item kind is the + # fabricated-number bug this route is being opened up to avoid, and accepting + # it would only move the fabrication into the store; a crew-level kind WITH a + # number would file a queue sweep under an issue it never touched, which is the + # same lie in the other direction. + _sweep = crew_store.CREW_LEVEL_EVENT_KIND + if crew_level and event_kind != _sweep: + return web.json_response( + {"error": (f"'number' is required for event_kind {event_kind!r} — only " + f"{_sweep} records a step with no issue"), + "code": "number_required"}, + status=400, + ) + if not crew_level and event_kind == _sweep: + return web.json_response( + {"error": (f"event_kind {event_kind!r} is crew-level and takes no " + "'number' — it records a step that belongs to no issue"), + "code": "unexpected_number"}, + status=400, + ) _crew, missing = await _require_crew(key, crew_id, must_be_live=True) if missing is not None: return missing + if crew_level: + # Refused rather than dropped. Every field named here patches a WORK ITEM, + # and this call creates none, so honouring the write while discarding them + # would report success for a phase move or a CI reading that was never + # stored anywhere. `skip_scope` is named alongside them because a pass is a + # decision about one issue and is indexed by that issue's number. + stray = sorted( + field for field in (*_WORK_PATCH_FIELDS, "skip_scope") if field in body + ) + if stray: + return web.json_response( + {"error": (f"{', '.join(repr(f) for f in stray)} belong to a work item, " + "so they cannot be sent without a 'number'"), + "code": "item_fields_without_number"}, + status=400, + ) + checkpoint = await routes._st( + key, + crew_store.record_crew_checkpoint, + key.owner, + key.repo, + crew_id, + event_text, + ) + return web.json_response(checkpoint) + + # Parsed here rather than above so the log line is validated FIRST, which is + # the order this route's docstring prescribes, and so the numbered path holds a + # plain ``int`` with no sentinel standing in for the crew-level case. + # + # Reusing the PR field parser rather than copying its bound: the bound is the + # point (the number becomes a FILENAME), and a second copy is how one of them + # ships without it. Its out-of-range text says "pull-request number", which is + # cosmetically wrong here and only reachable past 1e9. + number, number_error = routes._pr_number_field(body) + if number_error is not None: + return number_error + patch = {field: body[field] for field in _WORK_PATCH_FIELDS if field in body} # The route derives the pass's PROSE (only it has the request body); the store # owns the coupling — a non-`None` reason is what makes the transaction index diff --git a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_store.py b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_store.py index 9e1db5b499e..be971d8d0d9 100644 --- a/src/kiro_crew/apps/builtins/issue_radar/backend/crew_store.py +++ b/src/kiro_crew/apps/builtins/issue_radar/backend/crew_store.py @@ -101,11 +101,27 @@ TTL_ACTIVE_PHASES = frozenset({"claimed", "investigating", "implementing"}) EDITING_PHASES = frozenset({"implementing", "addressing-review"}) +#: ``sweep`` is the one kind that does NOT belong to an issue: it records that the +#: crew looked at the queue and took nothing, which is the only step in the +#: protocol with no work item behind it. Every other kind names something done TO +#: an item, so those lines carry a number and ``sweep`` lines do not (see +#: :func:`append_event`). Without it a crew that found an empty queue could only +#: report the cycle by attributing it to some issue it did not act on. EVENT_KINDS = ( "claim", "investigate", "reply", "implement", "ci", - "review", "conflict", "merge", "handback", "skip", "yield", + "review", "conflict", "merge", "handback", "skip", "yield", "sweep", ) +#: The one kind that records a crew-level step rather than one issue's. It is the +#: only kind :func:`append_event` accepts without a number, and the only one the +#: write route accepts with no work-item patch. A named constant rather than a set +#: of one: the frontend already tests `kind === 'sweep'` by equality, and a +#: collection whose only justification is a second member that does not exist +#: reads as generality this app has not earned. A second crew-level kind turns +#: these three equality tests into a membership test then, against a set that has +#: two real members. +CREW_LEVEL_EVENT_KIND = "sweep" + #: Why an issue was passed over, as a closed vocabulary. Two things need it to be #: closed rather than free prose: a crew reads the recent-skip list to calibrate #: what this fleet does not take on, and a human scanning the index wants to see @@ -993,11 +1009,76 @@ def _editing_item( # ── event ledger ──────────────────────────────────────────────────────────── -def _event_id(ts: str, crew_id: str, number: int, kind: str, text: str) -> str: - raw = f"{ts}|{crew_id}|{number}|{kind}|{text}".encode() +def _event_id(ts: str, crew_id: str, number: int | None, kind: str, text: str) -> str: + """Content-addressed id for one ledger line. + + ``number`` is ``None`` for a crew-level line. It renders as the empty string + so the formula for a NUMBERED line is byte-identical to what it has always + been -- changing that would give every existing line a new id and defeat the + merge-on-read dedupe for the whole ledger. The empty string cannot collide + with a real number, so the two families stay distinct. + """ + shown = "" if number is None else int(number) + raw = f"{ts}|{crew_id}|{shown}|{kind}|{text}".encode() return hashlib.sha256(raw).hexdigest()[:16] +def _event_entry( + crew_id: str, + number: int | None, + kind: str, + text: str, + *, + phase: str | None = None, +) -> dict[str, Any]: + """Validate one ledger line and build it. The ONLY place a line is shaped. + + Both writers go through here -- :func:`append_event` for an issue's line and + :func:`record_crew_checkpoint` for a crew-level one -- so the vocabulary + check, the number/kind pairing and the key order are stated once. Two + builders would let the numberless line drift from the numbered one, which is + the drift the pairing exists to prevent. + + The pairing is enforced in BOTH directions: a numberless line must carry a + crew-level kind, and a crew-level kind must not carry a number. ``number`` is + typed ``int | None`` here and ``int`` on :func:`append_event`, so the + numberless case is unreachable through the public issue-line writer by type + as well as by this check. + + CALL THIS UNDER THE EVENTS LOCK, immediately before writing the line. It + stamps ``ts``, and the stamp must be taken in the same order the lines are + appended: a caller that built its entry first and then blocked on the lock + would write a line whose timestamp PRECEDES the line already above it. That + inverts file order against timestamp order, and the two readers disagree --- + :func:`_latest_crew_event` walks the file backwards, so it would keep + coalescing onto that trailing sweep, while the crew page sorts by ``ts`` and + would never see it as the newest line. The idle stretch would then stay + hidden for as long as the crew kept idling, which is precisely what the + ongoing-stretch wording exists to show. + """ + if kind not in EVENT_KINDS: + raise CrewStoreError(f"unknown event kind {kind!r}") + if number is None and kind != CREW_LEVEL_EVENT_KIND: + raise CrewStoreError(f"event kind {kind!r} needs an issue number") + if number is not None and kind == CREW_LEVEL_EVENT_KIND: + raise CrewStoreError(f"event kind {kind!r} is crew-level and takes no issue number") + ts = store._now_iso() + # Built in the original key order so a NUMBERED line serializes exactly as it + # always has; the numberless line simply omits the key in place. + entry: dict[str, Any] = { + "id": _event_id(ts, crew_id, number, kind, text), + "ts": ts, + "crew_id": crew_id, + } + if number is not None: + entry["number"] = int(number) + entry["kind"] = kind + entry["text"] = text + if phase is not None: + entry["phase"] = phase + return entry + + def append_event( owner: str, repo: str, @@ -1009,7 +1090,7 @@ def append_event( *, phase: str | None = None, ) -> dict[str, Any]: - """Append one progress line. + """Append one issue's progress line. The id is content-addressed so a duplicated line merges on read rather than conflicting — the same discipline as ops-mission-control's ledger, whose own @@ -1020,6 +1101,12 @@ def append_event( absolute paths, host names and anything else environment-specific out of it; worktree paths belong in the work item's own fields. + A line that belongs to NO issue is written by + :func:`record_crew_checkpoint`, not here: that case has to read the crew's + tail and decide whether to append at all, under the same lock hold. This + function therefore requires a number, and the shared builder refuses a + crew-level kind through it. + ``phase`` is the work item's phase AFTER this write, and it is the sole datum that makes a per-phase dwell fold possible (:func:`crew_routes` folds it). Recorded because ``kind`` is NOT it: :data:`EVENT_KINDS` is not 1:1 with @@ -1032,28 +1119,127 @@ def append_event( event id: two lines that differ only in the phase they record are still the same logged reason, and folding one in twice must still merge. """ - if kind not in EVENT_KINDS: - raise CrewStoreError(f"unknown event kind {kind!r}") - ts = store._now_iso() - entry: dict[str, Any] = { - "id": _event_id(ts, crew_id, int(number), kind, text), - "ts": ts, - "crew_id": crew_id, - "number": int(number), - "kind": kind, - "text": text, - } - if phase is not None: - entry["phase"] = phase - path = events_path(owner, repo, root) lock_path = crews_dir(owner, repo, root) / "events.lock" with open(lock_path, "w") as fd: with platform_compat.file_lock(fd.fileno(), exclusive=True): - with open(path, "a", encoding="utf-8") as out: - out.write(json.dumps(entry) + "\n") + entry = _event_entry(crew_id, number, kind, text, phase=phase) + _write_event_line(owner, repo, entry, root) return entry +def _write_event_line( + owner: str, repo: str, entry: dict[str, Any], root: Path | None = None +) -> None: + """Append one already-built line. CALLER MUST HOLD the events lock. + + Split out so a writer that has to READ the tail before deciding whether to + append can do both inside ONE lock hold (see :func:`record_crew_checkpoint`). + Re-entering :func:`append_event` there would take the lock on a second + descriptor and block on the hold this frame already has. + """ + with open(events_path(owner, repo, root), "a", encoding="utf-8") as out: + out.write(json.dumps(entry) + "\n") + + +def _latest_crew_event( + owner: str, repo: str, crew_id: str, root: Path | None = None +) -> dict[str, Any] | None: + """This crew's newest ledger line, or ``None``. CALLER MUST HOLD the lock. + + Reads through :func:`read_events`, so it inherits that function's tolerance of + a torn tail and its duplicate collapse rather than re-parsing the file here. + """ + recent = read_events(owner, repo, root, crew_id=crew_id, limit=1) + return recent[0] if recent else None + + +def record_crew_checkpoint( + owner: str, + repo: str, + crew_id: str, + event_text: str, + root: Path | None = None, +) -> dict[str, Any]: + """Append one crew-level ledger line -- a step that belongs to no issue. + + TAKES NO KIND. There is exactly one crew-level kind, and the route that calls + this already 400s anything else on the numberless path, so a parameter here + could only ever carry :data:`CREW_LEVEL_EVENT_KIND` -- a value to be re-checked + and rejected rather than a choice a caller makes. Writing the constant directly + removes the argument and the guard that policed it; :func:`_event_entry` still + enforces the number/kind pairing, so the shape is validated in one place either + way. + + Returns the same ``{"item", "event", "skip"}`` shape as + :func:`commit_work_progress`, with ``item`` and ``skip`` as ``None``, so the + write route answers one shape and no caller has to branch on which kind of + write it made. ``coalesced`` says whether a line was actually written. + + CONSECUTIVE SWEEPS COALESCE, and this is the whole reason the function reads + before it writes. "I checked and took nothing" is a recurring LATEST-VALUE + fact, not an event: an idle crew is nudged on a timer, so appending one line + per cycle would add an unbounded run of them. Every ledger read is capped and + discards the OLDEST line first (:data:`crew_routes._DEFAULT_EVENTS` is 200), so + a crew idling for a couple of hundred cycles would push its entire real work + history out of its own work log -- the feature would bury exactly what the log + exists to show. Recording the TRANSITION rather than the state is the same + discipline :func:`commit_work_progress` already applies to ``phase``, which is + stamped only when an item is created or actually moves. + + So the FIRST sweep after real work is written, and a sweep whose crew already + has one as its newest line is answered with that existing line. The crew still + sees its checkpoint acknowledged; the log just does not grow a duplicate. The + timestamp therefore marks when the idle stretch BEGAN, which is the more useful + of the two readings -- "nothing to take since 09:12" beats "nothing to take as + of one minute ago", and how long the stretch has run is the question a human + opening a quiet crew is asking. + + WHAT THE SURVIVING TIMESTAMP DOES NOT MEAN. It records when this crew last + REPORTED an empty queue, and nothing about the present: consecutive reports + fold, and a crew that stops -- an operator pause, a crash, a lost nudge timer + -- stops reporting without saying so, because nothing on the crew record + evidences liveness. So the ledger cannot distinguish a crew still checking from + one that quietly died mid-stretch, and the crew page therefore renders this line + as the past instant it is rather than as a claim about now. An earlier revision + qualified it as "checking since ...", which read as present-tense activity and + so masked exactly the failure an operator opens that page to notice. Closing + that properly needs a real last-seen datum, which is a separate change. + + Deliberately NOT a branch inside :func:`commit_work_progress`. That function + exists to make three durable writes all-or-nothing, and every line of its lock + ordering and rollback is about reconciling an item, a repo-wide skip entry and + a ledger line. A crew-level line has nothing to reconcile and nothing to roll + back, so threading a ``None`` number through that transaction would add + branches to the most order-sensitive code in the store to describe a case that + has no transaction in it. + + ``event_text`` BECOMES PUBLIC on the crew page under the same rule as any other + line -- see :func:`append_event`. Unlike an item line it is not rendered into a + claim comment, because a crew-level step has no issue to comment on. + """ + # ONE lock hold spans the read and the write. Checking the tail and appending + # under two separate holds is a check-then-act: two crew turns waking together + # would both see no trailing sweep and both append, which is the exact run of + # duplicates this exists to prevent. + # + # COST, stated because the hold is exclusive: the tail read goes through + # `read_events`, which reads the WHOLE repo-wide events file and then walks it + # backwards to the first matching line. Nothing compacts that file today, so + # the hold grows with total ledger volume, not with this crew's share of it. + # Coalescing is what keeps an idle crew from being the thing that grows it -- + # an idle crew appends once per stretch, not once per wake -- but a compaction + # story is still owed if the file becomes large enough to matter here. + lock_path = crews_dir(owner, repo, root) / "events.lock" + with open(lock_path, "w") as fd: + with platform_compat.file_lock(fd.fileno(), exclusive=True): + latest = _latest_crew_event(owner, repo, crew_id, root) + if latest is not None and latest.get("kind") == CREW_LEVEL_EVENT_KIND: + return {"item": None, "event": latest, "skip": None, "coalesced": True} + entry = _event_entry(crew_id, None, CREW_LEVEL_EVENT_KIND, event_text) + _write_event_line(owner, repo, entry, root) + return {"item": None, "event": entry, "skip": None, "coalesced": False} + + def read_events( owner: str, repo: str, diff --git a/src/kiro_crew/mcp_tools/apps.py b/src/kiro_crew/mcp_tools/apps.py index f8c657170fc..1d4fafb2d7b 100644 --- a/src/kiro_crew/mcp_tools/apps.py +++ b/src/kiro_crew/mcp_tools/apps.py @@ -202,6 +202,11 @@ def schemas() -> list[dict[str, Any]]: "the real explanation in `why`, which is what the next crew " "reads. " "The crew and repo come from this session, not from arguments. " + "To record that you checked the queue and took NOTHING, send " + "`event_kind: sweep` with NO `number` — never invent an issue " + "number for a cycle you did no work in. That writes one " + "crew-level line and no work item, so it also takes none of the " + "work-item fields. " "WARNING — `event` and `why` BECOME PUBLIC: they are rendered into " "your claim comment on the forge as well as on your crew page. " "Never " @@ -216,7 +221,11 @@ def schemas() -> list[dict[str, Any]]: "properties": { "number": { "type": "integer", - "description": "Issue number this step belongs to", + "description": ( + "Issue number this step belongs to. Omit it ONLY with " + "`event_kind: sweep`, to record that you checked the " + "queue and took nothing — never invent a number for that" + ), }, "phase": { "type": "string", @@ -309,7 +318,13 @@ def schemas() -> list[dict[str, Any]]: "description": "Which kind of step this line records", }, }, - "required": ["number"], + # Nothing is unconditionally required. `number` was, which left a + # crew that swept an empty queue no way to record the cycle without + # inventing an issue number; the backend now pairs a missing number + # with the crew-level `sweep` kind and refuses every other + # combination, so the coupling is enforced where the write happens + # rather than by a schema that cannot express "one or the other". + "required": [], }, }, ] @@ -466,8 +481,13 @@ def issue_radar_crew_record(name: str, args: dict[str, Any]) -> str: "owner": _cw_owner, "repo": _cw_repo, "crew_id": _cw_crew_id, - "number": args["number"], } + # Presence, not truthiness. Omitting `number` is a MEANING — "this step belongs + # to no issue" — so it is forwarded as an absence rather than as a zero, which + # the route would read as a malformed issue number. The route pairs the absence + # with the crew-level `sweep` kind and refuses any other combination. + if "number" in args: + _cw_body["number"] = args["number"] # Local-only resume fields, passed through verbatim. NOT scrubbed: an # absolute worktree path is the point of the field, and it is never # rendered into a comment (crew_store keeps these local). @@ -536,9 +556,29 @@ def issue_radar_crew_record(name: str, args: dict[str, Any]) -> str: return f"Error: {_cw_resp['error']}" _cw_raw_item = _cw_resp.get("item") _cw_item: dict[str, Any] = _cw_raw_item if isinstance(_cw_raw_item, dict) else {} - _cw_ref = f"{_cw_owner}/{_cw_repo}#{args['number']}" - _cw_phase = _cw_item.get("phase") or _cw_body.get("phase") or "(phase unchanged)" - _cw_lines = [f"Recorded {_cw_ref}: phase `{_cw_phase}`."] + # A crew-level line has no issue to name and no item to report a phase for, so + # it is summarised by the crew it belongs to. Reusing the numbered wording here + # would print a bare `#` and claim a phase the write never touched. + if "number" in args: + _cw_ref = f"{_cw_owner}/{_cw_repo}#{args['number']}" + _cw_phase = _cw_item.get("phase") or _cw_body.get("phase") or "(phase unchanged)" + _cw_lines = [f"Recorded {_cw_ref}: phase `{_cw_phase}`."] + else: + # Whether a line was actually written matters to the caller: a coalesced + # checkpoint is still acknowledged, but a crew that believed each idle + # cycle added a line would misread its own log's length as its cycle count. + if _cw_resp.get("coalesced"): + _cw_lines = [ + f"Crew-level step for {_cw_owner}/{_cw_repo} folded into the open " + "idle stretch (its ledger line already says the queue was empty, " + "so no duplicate was added -- the existing line's timestamp marks " + "when the stretch began)." + ] + else: + _cw_lines = [ + f"Recorded a crew-level step for {_cw_owner}/{_cw_repo} " + "(no issue -- no work item was written)." + ] if _cw_body.get("event"): # Echo the stored line, not the argument — if a sanitizer pass # changed it, the crew must see what actually became public. diff --git a/src/kiro_crew/validation.py b/src/kiro_crew/validation.py index 24c4b77d339..c9f80db66d8 100644 --- a/src/kiro_crew/validation.py +++ b/src/kiro_crew/validation.py @@ -2035,6 +2035,11 @@ def _validate_omc_api(cleaned: dict[str, Any]) -> None: "handback", "skip", "yield", + # The one crew-level kind: the crew looked at the queue and took nothing. + # It is the only kind valid with NO ``number``, and it is invalid WITH one + # — a relation between two fields, so it is enforced on the write route + # and in the store rather than here. + "sweep", } ) #: Mirrors ``crew_store.SKIP_SCOPES`` — the classification a crew attaches to a @@ -2107,7 +2112,16 @@ def _validate_crew_record_couples_phase_to_an_event(args: dict[str, Any]) -> Non # Bounds the number that becomes the work item's FILENAME # (``crews//.json``) — same ENAMETOOLONG rationale as the # investigation record, hence the same constant. - FieldSpec("number", int, required=True, min_val=1, max_val=_ISSUE_RADAR_MAX_ITEM_NUMBER), + # + # NOT required. A crew that swept its queue and took nothing has no issue + # to name, and requiring one here left it recording the cycle against an + # issue it never acted on. The coupling that replaces the requirement — + # a missing number is valid ONLY with the crew-level ``sweep`` kind, and + # ``sweep`` is valid ONLY without one — is enforced on the write route and + # in the store, because it is a relation between two fields and this + # schema validates them one at a time. Keeping the bound here still + # matters: when a number IS sent it is the filename. + FieldSpec("number", int, min_val=1, max_val=_ISSUE_RADAR_MAX_ITEM_NUMBER), FieldSpec("phase", str, max_len=32, allowed=_ISSUE_RADAR_CREW_PHASES), # Bounded but deliberately NOT ``allowed=``, unlike ``phase`` beside it. # An out-of-vocabulary phase has to be refused — it would corrupt the diff --git a/temp-screenshots/crew-empty-queue/work-log-sweep.png b/temp-screenshots/crew-empty-queue/work-log-sweep.png new file mode 100644 index 00000000000..353fb037fa4 Binary files /dev/null and b/temp-screenshots/crew-empty-queue/work-log-sweep.png differ diff --git a/test/test_issue_radar_crew_mcp_tools.py b/test/test_issue_radar_crew_mcp_tools.py index 6f0dce848ed..beaecb5a272 100644 --- a/test/test_issue_radar_crew_mcp_tools.py +++ b/test/test_issue_radar_crew_mcp_tools.py @@ -216,11 +216,23 @@ def test_read_takes_no_arguments_at_all(self): assert spec["inputSchema"]["properties"] == {} assert MCP_CORE_SCHEMAS[READ_TOOL].fields == [] - def test_record_requires_only_the_issue_number(self): + def test_record_requires_nothing_unconditionally(self): + # `number` USED to be required, which left a crew that swept an empty + # queue no way to record the cycle without inventing an issue number. The + # coupling that replaced the requirement (a missing number is valid only + # with `sweep`, and `sweep` only without one) is a relation between two + # fields, which neither schema can express — it lives on the write route + # and in the store, so both schemas must agree that nothing is required. required = {f.name for f in MCP_CORE_SCHEMAS[RECORD_TOOL].fields if f.required} - assert required == {"number"} + assert required == set() spec = next(t for t in mcp_core._list_tools() if t["name"] == RECORD_TOOL) - assert spec["inputSchema"]["required"] == ["number"] + assert spec["inputSchema"]["required"] == [] + + def test_record_advertises_the_crew_level_sweep_kind(self): + # Advertised, or the model cannot discover the one kind that lets it + # report a cycle it did no work in. + spec = next(t for t in mcp_core._list_tools() if t["name"] == RECORD_TOOL) + assert "sweep" in spec["inputSchema"]["properties"]["event_kind"]["enum"] def test_record_advertises_no_identity_arguments(self): props = next( @@ -493,6 +505,65 @@ def test_the_local_resume_fields_survive_verbatim(self): assert body["base_sha"] == "f2aa4c8bb" +class TestACrewCanReportAnEmptyQueue(unittest.TestCase): + """The numberless call — `event_kind: sweep` with no `number`. + + `number` was the tool's one required field, so a crew that checked its queue + and took nothing could only record the cycle by naming an issue it never + acted on. These assert the absence travels as an ABSENCE: the route reads a + missing key as "this step has no issue", and a `0` standing in for it would + be rejected as a malformed issue number instead. + """ + + @staticmethod + def _sweep(put_result: dict | None = None, **over): + args = {"event": "checked 42 open issues, took none", "event_kind": "sweep"} + args.update(over) + cleaned = validate_tool_args(args, MCP_CORE_SCHEMAS[RECORD_TOOL]) + captured: dict = {} + + def fake_put(path, body=None, session_key=None): + captured["body"] = body + return put_result if put_result is not None else { + "item": None, "skip": None, + "event": {"text": body.get("event") or ""}, + } + + with patch.object( + mcp_core, "_resolve_session_key_strict", return_value="crew-c_7f3a" + ): + with patch.object(mcp_core, "_get", return_value=CREW_PAYLOAD): + with patch.object(mcp_core, "_put", side_effect=fake_put): + out = mcp_core._call_tool_inner(RECORD_TOOL, cleaned) + return captured, out + + def test_a_sweep_validates_without_a_number(self): + cleaned = validate_tool_args( + {"event": "queue empty", "event_kind": "sweep"}, + MCP_CORE_SCHEMAS[RECORD_TOOL], + ) + assert "number" not in cleaned + + def test_the_number_key_is_omitted_from_the_request(self): + captured, _ = self._sweep() + # Omitted, not zero: the route reads a present-but-invalid number as a + # 400, so sending 0 would fail the write rather than record the sweep. + assert "number" not in captured["body"] + assert set(captured["body"]) == {"owner", "repo", "crew_id", "event", "event_kind"} + + def test_the_summary_names_the_crew_rather_than_a_bare_hash(self): + _, out = self._sweep() + assert "#" not in out.splitlines()[0] + assert "no issue" in out + assert "checked 42 open issues, took none" in out + + def test_a_numbered_call_still_sends_and_reports_its_number(self): + # The guard is on presence, so the ordinary path must be untouched. + captured, out = _record(number=12, event="took it", event_kind="claim") + assert captured["body"]["number"] == 12 + assert "#12" in out + + class TestCiStateAssembly(unittest.TestCase): """The flat ci_* args become the store's ``ci_state`` dict.""" diff --git a/test/test_issue_radar_crew_routes.py b/test/test_issue_radar_crew_routes.py index 2f80ad55095..d9854f31ca2 100644 --- a/test/test_issue_radar_crew_routes.py +++ b/test/test_issue_radar_crew_routes.py @@ -726,6 +726,144 @@ async def test_a_malformed_agent_body_is_still_400_not_403(self): self.assertEqual(_payload(res)["code"], "invalid_json") +# ── a step that belongs to no issue ───────────────────────────────────────── + + +class TestACrewCanRecordAnEmptyQueue(_CrewRouteCase): + """``PUT /crew/work`` with NO ``number`` — the empty-queue checkpoint. + + A crew that swept its queue and took nothing had no way to record the cycle: + the route required a number because the number becomes a work-item filename, + so the only way to report "checked, nothing to take" was to attribute it to an + issue the crew never acted on. The ledger therefore either lied or stayed + silent, and a resumed crew could not tell the two apart. + + The pairing is asserted in BOTH directions. Accepting an item kind with no + number would only move the fabrication into the store, and accepting the + crew-level kind WITH a number files a sweep under an issue it never touched — + the same false attribution written the other way round. + """ + + def agent(self, crew: dict) -> str: + return f"dashboard:{crew['slot_key']}" + + async def test_a_sweep_needs_no_number_and_writes_no_work_item(self): + crew = self.crew("Andromeda") + res = await self.call( + "PUT", "/crew/work", + body={"event": "checked 42 open issues, took none", "event_kind": "sweep"}, + internal_auth=True, session=self.agent(crew), + ) + self.assertEqual(res.status, 200) + body = _payload(res) + # Same envelope as a numbered write, so the MCP tool reads one shape; + # `coalesced` rides alongside it to say whether a line was written. + self.assertTrue({"item", "event", "skip"}.issubset(set(body))) + self.assertIsNone(body["item"]) + self.assertIsNone(body["skip"]) + self.assertFalse(body["coalesced"]) + self.assertEqual( + [e["text"] for e in self.ledger(crew["id"])], + ["checked 42 open issues, took none"], + ) + # Absent, not zero — a `0` would read as a real issue everywhere. + self.assertNotIn("number", self.ledger(crew["id"])[0]) + self.assertEqual(crew_store.list_work_items(OWNER, REPO, crew["id"], self.root), []) + + async def test_an_idle_crew_does_not_bury_its_own_work_log(self): + """Repeated idle cycles must not run the ledger away. + + Every ledger read is capped and drops the OLDEST line first, so one line + per nudge would push the crew's real history out of the log this feature + exists to make honest. + """ + crew = self.crew("Andromeda") + body = {"event": "queue empty", "event_kind": "sweep"} + first = await self.call( + "PUT", "/crew/work", body=body, internal_auth=True, session=self.agent(crew), + ) + again = await self.call( + "PUT", "/crew/work", body={**body, "event": "still empty"}, + internal_auth=True, session=self.agent(crew), + ) + self.assertEqual((first.status, again.status), (200, 200)) + self.assertFalse(_payload(first)["coalesced"]) + self.assertTrue(_payload(again)["coalesced"]) + # One line, and it is the FIRST one -- its timestamp marks when the idle + # stretch began, which is what a human opening a quiet crew wants to know. + self.assertEqual([e["text"] for e in self.ledger(crew["id"])], ["queue empty"]) + + async def test_an_item_kind_without_a_number_is_refused(self): + crew = self.crew("Andromeda") + res = await self.call( + "PUT", "/crew/work", + body={"event": "took something", "event_kind": "claim"}, + internal_auth=True, session=self.agent(crew), + ) + self.assertEqual(res.status, 400) + self.assertEqual(_payload(res)["code"], "number_required") + self.assertEqual(self.ledger(crew["id"]), []) + + async def test_a_sweep_carrying_a_number_is_refused(self): + crew = self.crew("Andromeda") + res = await self.call( + "PUT", "/crew/work", + body={"number": 7, "event": "queue empty", "event_kind": "sweep"}, + internal_auth=True, session=self.agent(crew), + ) + self.assertEqual(res.status, 400) + self.assertEqual(_payload(res)["code"], "unexpected_number") + self.assertEqual(self.ledger(crew["id"]), []) + + async def test_a_present_but_invalid_number_is_not_read_as_a_sweep(self): + """A typo must stay a 400, not become "this step has no issue".""" + crew = self.crew("Andromeda") + res = await self.call( + "PUT", "/crew/work", + body={"number": 0, "event": "took #0", "event_kind": "claim"}, + internal_auth=True, session=self.agent(crew), + ) + self.assertEqual(res.status, 400) + self.assertEqual(_payload(res)["code"], "invalid_number") + self.assertEqual(self.ledger(crew["id"]), []) + + async def test_work_item_fields_are_refused_rather_than_dropped(self): + """Honouring the line while discarding the patch would report a phase + move that was never stored anywhere.""" + crew = self.crew("Andromeda") + res = await self.call( + "PUT", "/crew/work", + body={"event": "queue empty", "event_kind": "sweep", + "phase": "claimed", "ci_state": {"state": "success"}}, + internal_auth=True, session=self.agent(crew), + ) + self.assertEqual(res.status, 400) + self.assertEqual(_payload(res)["code"], "item_fields_without_number") + self.assertIn("'ci_state'", _payload(res)["error"]) + self.assertIn("'phase'", _payload(res)["error"]) + self.assertEqual(self.ledger(crew["id"]), []) + + async def test_a_sweep_still_needs_a_reason(self): + crew = self.crew("Andromeda") + res = await self.call( + "PUT", "/crew/work", body={"event_kind": "sweep"}, + internal_auth=True, session=self.agent(crew), + ) + self.assertEqual(res.status, 400) + self.assertEqual(_payload(res)["code"], "event_required") + + async def test_a_sweep_from_a_non_crew_session_is_refused(self): + """The numberless path must not become a way around the identity gate.""" + self.crew("Andromeda") + res = await self.call( + "PUT", "/crew/work", + body={"event": "queue empty", "event_kind": "sweep"}, + internal_auth=True, session="dashboard:chat-3-1730000000", + ) + self.assertEqual(res.status, 403) + self.assertEqual(_payload(res)["code"], "not_a_crew_session") + + # ── which routes an agent may reach at all ────────────────────────────────── diff --git a/test/test_issue_radar_crew_store.py b/test/test_issue_radar_crew_store.py index a163fb9597a..b92f953720c 100644 --- a/test/test_issue_radar_crew_store.py +++ b/test/test_issue_radar_crew_store.py @@ -30,10 +30,12 @@ every function for exactly this reason, so nothing here touches a real data home. """ +import inspect import json import math import os import threading +import time from contextlib import contextmanager import pytest @@ -588,6 +590,219 @@ def test_unknown_event_kind_is_refused(tmp_path): cs.append_event(OWNER, REPO, crew["id"], 1, "vibes", "…", tmp_path) +# ── crew-level lines (a step that belongs to no issue) ────────────────────── +# +# The invariant these protect is the one the feature exists for: a crew that +# checked the queue and took nothing must be able to SAY so. Before this, the +# only way to record the cycle was to attribute it to an issue the crew never +# acted on, so the ledger either lied or stayed silent. Both directions of the +# pairing are asserted, because a crew-level kind carrying a number is the same +# false attribution written the other way round. + + +def test_a_crew_level_line_omits_the_number_entirely(tmp_path): + crew = _crew(tmp_path) + result = cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "queue empty", tmp_path) + entry = result["event"] + # Absent, NOT zero: a `0` is indistinguishable from a real issue number in + # every filter and join that keys on this field. + assert "number" not in entry + stored = cs.read_events(OWNER, REPO, tmp_path) + assert len(stored) == 1 + assert "number" not in stored[0] + assert (stored[0]["kind"], stored[0]["text"]) == ("sweep", "queue empty") + + +def test_an_item_kind_without_a_number_is_refused(tmp_path): + # Asserted on the shared builder, which is where the pairing lives: the public + # issue-line writer types `number` as `int`, so this case cannot reach it. + with pytest.raises(cs.CrewStoreError, match="needs an issue number"): + cs._event_entry("c_1", None, "claim", "claimed") + + +def test_a_crew_level_kind_with_a_number_is_refused(tmp_path): + crew = _crew(tmp_path) + with pytest.raises(cs.CrewStoreError, match="takes no issue number"): + cs.append_event(OWNER, REPO, crew["id"], 7, "sweep", "queue empty", tmp_path) + + +def test_only_one_writer_shapes_a_numberless_line(tmp_path): + """`append_event` must NOT be a second way to write a crew-level line. + + Coalescing has to read the crew's tail and decide whether to append at all, + under one lock hold, so the crew-level path cannot delegate to + `append_event`. That left two writers for the same shape, one of which no + production code called -- so the issue-line writer now requires a number and + the pairing check in the shared builder refuses a crew-level kind through it. + """ + crew = _crew(tmp_path) + with pytest.raises(cs.CrewStoreError, match="takes no issue number"): + cs.append_event(OWNER, REPO, crew["id"], 1, cs.CREW_LEVEL_EVENT_KIND, "x", tmp_path) + # And the one legitimate writer still produces the same shape the builder makes. + built = cs._event_entry(crew["id"], None, cs.CREW_LEVEL_EVENT_KIND, "queue empty") + written = cs.record_crew_checkpoint( + OWNER, REPO, crew["id"], "queue empty", tmp_path + )["event"] + assert set(built) == set(written) + + +def test_a_numbered_line_keeps_its_historical_id(tmp_path): + """The id formula for a NUMBERED line must not have changed. + + It is content-addressed and drives merge-on-read dedupe, so a new formula + would give every existing line a fresh id and silently defeat the dedupe for + the whole ledger. Asserted against the literal formula rather than against a + golden string so the test does not need a frozen clock. + """ + crew = _crew(tmp_path) + entry = cs.append_event(OWNER, REPO, crew["id"], 4, "claim", "claimed", tmp_path) + assert entry["id"] == cs._event_id(entry["ts"], crew["id"], 4, "claim", "claimed") + # And the two families cannot collide: the numberless variant renders the + # number as the empty string, which no real number produces. + assert cs._event_id(entry["ts"], crew["id"], None, "sweep", "x") != cs._event_id( + entry["ts"], crew["id"], 0, "sweep", "x" + ) + + +def test_record_crew_checkpoint_writes_one_line_and_no_item(tmp_path): + crew = _crew(tmp_path) + result = cs.record_crew_checkpoint( + OWNER, REPO, crew["id"], "checked 42 open issues, took none", tmp_path + ) + # Same envelope as commit_work_progress, so the write route answers one shape. + assert {"item", "event", "skip"} <= set(result) + assert result["item"] is None and result["skip"] is None + assert result["event"]["kind"] == "sweep" + assert result["coalesced"] is False + # No work item was created, so nothing consumes a work slot. + assert cs.list_work_items(OWNER, REPO, crew["id"], tmp_path) == [] + + +def test_consecutive_sweeps_coalesce_instead_of_running_away(tmp_path): + """An idle crew must not bury its own work log. + + A sweep is a recurring latest-value fact and the crew is nudged on a timer, so + one line per cycle would be an unbounded run. Every ledger read is capped and + drops the OLDEST line first, so a few hundred idle cycles would push the real + history out of the log the feature exists to make honest. + """ + crew = _crew(tmp_path) + first = cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "queue empty", tmp_path) + again = cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "still empty", tmp_path) + + assert first["coalesced"] is False + assert again["coalesced"] is True + # The SECOND call is answered with the FIRST line, so the timestamp marks when + # the idle stretch began rather than when it was last observed. + assert again["event"]["id"] == first["event"]["id"] + assert again["event"]["text"] == "queue empty" + assert len(cs.read_events(OWNER, REPO, tmp_path)) == 1 + + +def test_a_sweep_after_real_work_is_written(tmp_path): + # Coalescing keys on the crew's newest line, so the transition back into idle + # is recorded -- otherwise the guard would swallow every sweep after the first. + crew = _crew(tmp_path) + cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "queue empty", tmp_path) + cs.append_event(OWNER, REPO, crew["id"], 7, "claim", "took #7", tmp_path) + third = cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "empty again", tmp_path) + + assert third["coalesced"] is False + assert [e["kind"] for e in cs.read_events(OWNER, REPO, tmp_path)] == ["sweep", "claim", "sweep"] + + +def test_a_sweep_blocked_on_the_lock_is_stamped_after_the_line_it_follows(tmp_path): + """File order and timestamp order must agree, so the two readers agree. + + They disagree when it does not: :func:`_latest_crew_event` walks the file + backwards, so it keeps coalescing onto a trailing sweep, while the crew page + sorts by ``ts`` and never surfaces that sweep as the newest line. The idle + stretch then stays hidden for as long as the crew keeps idling -- exactly what + the ongoing-stretch wording exists to show. A writer that stamped its entry and + THEN blocked on the lock produced that inversion, so the stamp is taken under + the hold. + """ + crew = _crew(tmp_path) + lock_path = cs.crews_dir(OWNER, REPO, tmp_path) / "events.lock" + started = threading.Event() + + def _sweep(): + started.set() + cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "queue empty", tmp_path) + + worker = threading.Thread(target=_sweep) + with open(lock_path, "w") as fd: + with cs.platform_compat.file_lock(fd.fileno(), exclusive=True): + worker.start() + assert started.wait(5) + # The sweep is now waiting on this hold. Append the numbered line that + # won the race: a stamp taken before the wait would predate it. + time.sleep(0.05) + cs._write_event_line( + OWNER, REPO, cs._event_entry(crew["id"], 7, "claim", "took #7"), tmp_path + ) + worker.join(5) + assert not worker.is_alive() + + # read_events is newest-first, so the sweep -- appended last -- comes first and + # must carry the LATER stamp. + stored = cs.read_events(OWNER, REPO, tmp_path) + assert [e["kind"] for e in stored] == ["sweep", "claim"] + assert stored[0]["ts"] >= stored[1]["ts"] + + +def test_a_pause_does_not_break_the_fold_because_liveness_is_not_evidenced(tmp_path): + """Coalescing is deliberately blind to pause state -- see the docstring. + + An earlier revision stamped ``paused_at`` and refused to fold across it, to stop + the page claiming an unbroken "checking since" run over a stop. That claim is + gone: the row now renders as a past instant, so the fold has nothing to protect, + and the field was removed rather than kept to cover one of the three stop modes + while a crash and a lost timer stayed uncovered. This pins the simpler rule so + it is not re-complicated without a reason. + """ + crew = _crew(tmp_path) + first = cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "queue empty", tmp_path) + cs.set_crew_paused(OWNER, REPO, crew["id"], True, "operator stopped it", tmp_path) + cs.set_crew_paused(OWNER, REPO, crew["id"], False, "", tmp_path) + after = cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "empty again", tmp_path) + + assert first["coalesced"] is False + assert after["coalesced"] is True + assert after["event"]["id"] == first["event"]["id"] + assert len(cs.read_events(OWNER, REPO, tmp_path)) == 1 + # The record carries no pause timestamp: the field is gone, not merely unused. + assert "paused_at" not in cs.read_crew(OWNER, REPO, crew["id"], tmp_path) + + +def test_one_crews_sweep_does_not_coalesce_anothers(tmp_path): + # The ledger file is repo-wide, so the tail read must be crew-scoped or one + # crew's idle line would silently swallow another crew's. + a = _crew(tmp_path, name="Andromeda") + b = _crew(tmp_path, name="Whirlpool") + cs.record_crew_checkpoint(OWNER, REPO, a["id"], "a is empty", tmp_path) + theirs = cs.record_crew_checkpoint(OWNER, REPO, b["id"], "b is empty", tmp_path) + + assert theirs["coalesced"] is False + assert len(cs.read_events(OWNER, REPO, tmp_path)) == 2 + + +def test_the_checkpoint_writer_takes_no_kind_so_a_wrong_one_is_unrepresentable(tmp_path): + """The refusal became structural, which is why the guard that raised is gone. + + An earlier revision took an ``event_kind`` and raised on anything but the + crew-level one. The route already 400s a non-crew-level kind on the numberless + path, so the argument could only ever carry the one value -- passing it existed + to be re-checked. Now the constant is written directly and a caller has nothing + to get wrong. The number/kind pairing is still enforced, in ``_event_entry``, + which the tests above cover in both directions. + """ + crew = _crew(tmp_path) + assert "event_kind" not in inspect.signature(cs.record_crew_checkpoint).parameters + written = cs.record_crew_checkpoint(OWNER, REPO, crew["id"], "queue empty", tmp_path) + assert written["event"]["kind"] == cs.CREW_LEVEL_EVENT_KIND + + # ── phase classification ──────────────────────────────────────────────────── diff --git a/website/capture/crew-work-log-sweep.html b/website/capture/crew-work-log-sweep.html new file mode 100644 index 00000000000..3d269df3bac --- /dev/null +++ b/website/capture/crew-work-log-sweep.html @@ -0,0 +1,11 @@ + + + + + crew work log sweep capture + + +
+ + + diff --git a/website/capture/crew-work-log-sweep.tsx b/website/capture/crew-work-log-sweep.tsx new file mode 100644 index 00000000000..475cbc3b02a --- /dev/null +++ b/website/capture/crew-work-log-sweep.tsx @@ -0,0 +1,136 @@ +/** + * Evidence for the crew work log's crew-level line. + * + * WHAT CHANGED: a crew that swept its queue and took nothing can now record the + * cycle. That line belongs to no issue, so the backend omits `number` from it + * entirely. The Issue column is monospaced and right-aligned against real issue + * numbers, so rendering the absent value straight would print a bare `#` and + * read as a number that failed to load; it renders an em dash instead, and the + * OUTCOME badge carries the new `sweep` kind. + * + * WHY A HARNESS: the work log lives inside `CrewPageView`, which needs the whole + * Issue Radar context and a live gateway to reach. The page's only input is + * `GET /api/apps/issue-radar/crew`, so stubbing that one response renders the + * REAL view -- real classes, real Tailwind output, real theme tokens, real + * catalog -- with nothing re-implemented here. The frame therefore shows what + * ships, including the numbered rows beside the crew-level one, which is the + * comparison the change is about. + * + * Theme via query string: ?theme=dark|light + */ +import { createRoot } from 'react-dom/client' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { Provider } from 'react-redux' +import { MemoryRouter } from 'react-router-dom' + +import type { CrewDetailResponse } from '../src/apps/issue-radar/api' +import CrewPageView from '../src/apps/issue-radar/views/CrewPageView' +import { IssueRadarProvider } from '../src/apps/issue-radar/context' +import { initI18n } from '../src/i18n/all' +import { store } from '../src/store' +import '../src/index.css' + +const params = new URLSearchParams(location.search) +const theme = params.get('theme') === 'light' ? 'light' : 'dark' +document.documentElement.setAttribute('data-theme', theme === 'light' ? 'kiro-light' : 'kiro-dark') + +const OWNER = 'kirodotdev' +const REPO = 'Kiro' +const CREW = 'c_1a2b3c4d' + +/** An ISO stamp `hours` before now, so every row lands inside the 24h window and + * the Earlier divider stays out of the frame. */ +const ago = (hours: number) => new Date(Date.now() - hours * 3_600_000).toISOString() + +const PAYLOAD: CrewDetailResponse = { + crew: { + schema: 1, + id: CREW, + name: 'Andromeda', + avatar_seed: CREW, + avatar_variant: 2, + agent: 'kirocrew', + model: 'claude-opus-5', + extra_prompt: '', + labels: ['area: apps'], + auto_resolve_conflicts: false, + auto_merge: false, + unattended: true, + max_open: 3, + worktree_root: '', + slot_key: `crew-${CREW}`, + enabled: true, + paused_reason: '', + created_at: ago(72), + retired_at: null, + }, + items: [ + { + schema: 1, crew_id: CREW, owner: OWNER, repo: REPO, number: 2251, + phase: 'awaiting-ci', outcome: null, decision: '', why: '', + next: 'Wait out CI round 3, then re-read the two inherited reds before rebasing.', + tried: [], worktree: '', branch: 'fix/safe-chmod-windows', base_sha: '', + pr_number: 2288, ci_state: { passed: 41, total: 47, round: 3 }, + claim_comment_id: null, labels_applied: [], + claimed_at: ago(30), last_progress_at: ago(1), finished_at: null, + }, + ], + events: [ + // The crew-level line: no `number` key at all, which is what the Issue cell + // has to render honestly. It is also the NEWEST line, so its stretch is still + // open and its WHEN column reads as ongoing. + { + id: 'ev-sweep', ts: ago(0.4), crew_id: CREW, kind: 'sweep', + text: 'checked 42 open issues, took none -- every candidate is claimed or skipped', + }, + // Numbered siblings, unchanged, so the frame shows the contrast rather than + // one row in isolation. + { id: 'ev-ci', ts: ago(1), crew_id: CREW, number: 2251, kind: 'ci', text: 'CI round 3 -- 41/47 green, 2 inherited from main' }, + { id: 'ev-impl', ts: ago(3), crew_id: CREW, number: 2251, kind: 'implement', text: 'added the Windows branch to _safe_chmod' }, + { id: 'ev-claim', ts: ago(6), crew_id: CREW, number: 2251, kind: 'claim', text: 'took it -- regression test already fails on main' }, + // A SECOND crew-level line, older than that work: this stretch has ENDED, so + // the same kind must render as a plain past instant. Both cases in one frame, + // because the qualifier is keyed on recency and a frame showing only the open + // stretch could not tell the two apart. + { + id: 'ev-sweep-closed', ts: ago(9), crew_id: CREW, kind: 'sweep', + text: 'checked 39 open issues, took none', + }, + ], + counts: { open: 1 }, +} + +// Only the crew endpoint is stubbed; anything else falls through so a missed +// dependency surfaces as a real network error instead of an empty render. +const realFetch = window.fetch.bind(window) +window.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url + if (url.includes('/apps/issue-radar/crew?')) { + return Promise.resolve(new Response(JSON.stringify(PAYLOAD), { + status: 200, headers: { 'Content-Type': 'application/json' }, + })) + } + return realFetch(input as RequestInfo, init) +}) as typeof window.fetch + +const active = { owner: OWNER, repo: REPO, provider: 'github' as const, host: 'github.com' } + +initI18n('en') +createRoot(document.getElementById('root')!).render( + + + + {}} + onAddRepo={() => {}} + > +
+ +
+ + + + , +) diff --git a/website/scripts/capture-crew-work-log-sweep.mjs b/website/scripts/capture-crew-work-log-sweep.mjs new file mode 100644 index 00000000000..dc78f387499 --- /dev/null +++ b/website/scripts/capture-crew-work-log-sweep.mjs @@ -0,0 +1,49 @@ +/** + * Screenshot of the crew work log showing a crew-level `sweep` line beside + * numbered rows, via the capture/crew-work-log-sweep harness (which stubs only + * GET /api/apps/issue-radar/crew). Asserts the sweep row's Issue cell holds an em + * dash and no `#` before shooting -- the whole point of the frame -- so a harness + * that silently stopped rendering the row cannot produce a green screenshot. + * + * Usage: node scripts/capture-crew-work-log-sweep.mjs + */ +import { chromium } from 'playwright' + +const base = process.argv[2] || 'http://127.0.0.1:5199' +const out = process.argv[3] || '../temp-screenshots/crew-empty-queue/work-log-sweep.png' + +const b = await chromium.launch() +const p = await (await b.newContext({ viewport: { width: 1180, height: 900 }, deviceScaleFactor: 2 })).newPage() +await p.goto(`${base}/capture/crew-work-log-sweep.html?theme=dark`, { waitUntil: 'networkidle' }) + +const sweepRow = p.locator('[data-testid="work-log-row-ev-sweep"]') +await sweepRow.waitFor({ state: 'visible', timeout: 15_000 }) +const issueCell = sweepRow.locator('td').nth(1) +const cellText = (await issueCell.textContent())?.trim() +if (cellText !== '\u2014') { + throw new Error(`sweep row Issue cell is ${JSON.stringify(cellText)}, expected an em dash`) +} +const numbered = await p.locator('[data-testid="work-log-row-ev-ci"] td').nth(1).textContent() +if (numbered?.trim() !== '#2251') { + throw new Error(`numbered row Issue cell is ${JSON.stringify(numbered)}, expected #2251`) +} + +// Both sweep rows must read as PAST INSTANTS. Neither may carry a present-tense +// qualifier: nothing on this page evidences that a crew is alive, so a row claiming +// ongoing activity would misreport a crashed crew as a healthily idle one. +const newestWhen = (await sweepRow.locator('td').nth(0).textContent())?.trim() ?? '' +const olderWhen = ( + await p.locator('[data-testid="work-log-row-ev-sweep-closed"] td').nth(0).textContent() +)?.trim() ?? '' +for (const [label, text] of [['newest', newestWhen], ['older', olderWhen]]) { + if (/since/i.test(text)) { + throw new Error(`${label} sweep reads ${JSON.stringify(text)}, expected a bare timestamp`) + } + if (!text) { + throw new Error(`${label} sweep has an empty WHEN cell`) + } +} + +await p.locator('[data-testid="capture-frame"]').screenshot({ path: out }) +console.log(`captured ${out} (em dash, numbered sibling, and both sweeps as past instants)`) +await b.close() diff --git a/website/src/apps/issue-radar/api.ts b/website/src/apps/issue-radar/api.ts index 10cd68bd88a..7c0c7099c04 100644 --- a/website/src/apps/issue-radar/api.ts +++ b/website/src/apps/issue-radar/api.ts @@ -960,10 +960,13 @@ export const CREW_PHASES = [ export type CrewPhase = typeof CREW_PHASES[number] /** Mirrors `crew_store.EVENT_KINDS`. The store REFUSES an unknown kind, so this - * union is enforced server-side rather than merely documented. */ + * union is enforced server-side rather than merely documented. + * + * `sweep` is the one kind that belongs to no issue — a crew reporting that it + * checked the queue and took nothing — so its lines carry no `number`. */ export const CREW_EVENT_KINDS = [ 'claim', 'investigate', 'reply', 'implement', 'ci', - 'review', 'conflict', 'merge', 'handback', 'skip', 'yield', + 'review', 'conflict', 'merge', 'handback', 'skip', 'yield', 'sweep', ] as const export type CrewEventKind = typeof CREW_EVENT_KINDS[number] @@ -1090,7 +1093,10 @@ export interface CrewEvent { id: string ts: string crew_id: string - number: number + /** ABSENT on a crew-level line (`kind: 'sweep'`), which belongs to no issue. + * Optional rather than nullable because the backend omits the key entirely — + * a `0` would be indistinguishable from a real issue number. */ + number?: number kind: CrewEventKind text: string } diff --git a/website/src/apps/issue-radar/views/CrewPageView.tsx b/website/src/apps/issue-radar/views/CrewPageView.tsx index 9f7c00248fb..34a8b02dc9c 100644 --- a/website/src/apps/issue-radar/views/CrewPageView.tsx +++ b/website/src/apps/issue-radar/views/CrewPageView.tsx @@ -122,6 +122,7 @@ const KIND_LABEL_KEY: Record = { 'handback': 'apps.issueRadar.views.crews.page.kind_handback', 'skip': 'apps.issueRadar.views.crews.page.kind_skip', 'yield': 'apps.issueRadar.views.crews.page.kind_yield', + 'sweep': 'apps.issueRadar.views.crews.page.kind_sweep', } function phaseVariant(phase: CrewPhase): 'ok' | 'err' | 'warn' | 'aim' | 'muted' { @@ -139,9 +140,21 @@ function kindVariant(kind: CrewEventKind): 'ok' | 'err' | 'warn' | 'aim' | 'mute if (kind === 'merge') return 'ok' if (kind === 'ci' || kind === 'conflict') return 'warn' if (kind === 'skip' || kind === 'yield' || kind === 'handback') return 'muted' + // A sweep took no work, so it reads as background like the other did-not-act + // kinds rather than competing with the lines that moved something. + if (kind === 'sweep') return 'muted' return 'aim' } +/** The Issue cell's text for one ledger line. + * + * A crew-level line has no issue, and the honest cell is an em dash rather than + * `#` followed by nothing: the column is monospaced and right-aligned against + * numbers, so a bare `#` reads as a number that failed to load. */ +function issueCell(number: number | undefined): string { + return number === undefined ? '—' : `#${number}` +} + /** A calendar date with the year elided while it is THIS year — `Aug 6` now, * `Aug 6, 2025` once the year turns, so an old stamp can never read as recent. * Both widths come from `Intl` under the active UI locale (never the host's). */ @@ -525,7 +538,7 @@ export default function CrewPageView({ crewId, onEdit }: CrewPageViewProps) { {fmtRelative(e.ts, { now: log.nowMs })} - #{e.number} + {issueCell(e.number)} {e.text} {t(KIND_LABEL_KEY[e.kind])} @@ -544,7 +557,7 @@ export default function CrewPageView({ crewId, onEdit }: CrewPageViewProps) { {shortDate(e.ts, log.nowMs)} - #{e.number} + {issueCell(e.number)} {e.text} {t(KIND_LABEL_KEY[e.kind])} diff --git a/website/src/i18n/locales/bn.json b/website/src/i18n/locales/bn.json index ad1a9eac553..46c23347a93 100644 --- a/website/src/i18n/locales/bn.json +++ b/website/src/i18n/locales/bn.json @@ -2304,6 +2304,7 @@ "kind_reply": "উত্তর দিয়েছে", "kind_review": "রিভিউ", "kind_skip": "পরিধির বাইরে", + "kind_sweep": "পরীক্ষিত", "kind_yield": "ছেড়ে দিয়েছে", "no_next_recorded": "পরবর্তী ধাপ কিছু লেখা নেই", "no_open_work_items": "কিছুই চলছে না", diff --git a/website/src/i18n/locales/de.json b/website/src/i18n/locales/de.json index 8bd1e46b8b5..b24beb56a01 100644 --- a/website/src/i18n/locales/de.json +++ b/website/src/i18n/locales/de.json @@ -2348,6 +2348,7 @@ "kind_reply": "Geantwortet", "kind_review": "Review", "kind_skip": "Übergangen", + "kind_sweep": "Geprüft", "kind_yield": "Freigegeben", "no_next_recorded": "kein nächster Schritt vermerkt", "no_open_work_items": "Nichts in Arbeit", diff --git a/website/src/i18n/locales/en-XA.json b/website/src/i18n/locales/en-XA.json index 86cc1b0d19e..1c0c5784d17 100644 --- a/website/src/i18n/locales/en-XA.json +++ b/website/src/i18n/locales/en-XA.json @@ -2571,6 +2571,7 @@ "kind_reply": "[Ŕèþĺìèð ···········]", "kind_review": "[Ŕèṽìèẁ ·········]", "kind_skip": "[Þàşşèð ·········]", + "kind_sweep": "[Şẁèþţ ········]", "kind_yield": "[Ýìèĺðèð ···········]", "no_next_recorded": "[ñø ñèẋţ şţèþ ŕèçøŕðèð ···············]", "no_open_work_items": "[Ñøţĥìñğ ìñ ƒĺìğĥţ ···············]", diff --git a/website/src/i18n/locales/en.manual.json b/website/src/i18n/locales/en.manual.json index 02effd66ad7..6adc555818f 100644 --- a/website/src/i18n/locales/en.manual.json +++ b/website/src/i18n/locales/en.manual.json @@ -685,6 +685,7 @@ "kind_reply": "Replied", "kind_review": "Review", "kind_skip": "Passed", + "kind_sweep": "Swept", "kind_yield": "Yielded", "no_next_recorded": "no next step recorded", "no_open_work_items": "Nothing in flight", diff --git a/website/src/i18n/locales/es.json b/website/src/i18n/locales/es.json index d88d97db103..6cc8f302102 100644 --- a/website/src/i18n/locales/es.json +++ b/website/src/i18n/locales/es.json @@ -2354,6 +2354,7 @@ "kind_reply": "Respondida", "kind_review": "Revisión", "kind_skip": "Descartada", + "kind_sweep": "Revisado", "kind_yield": "Liberada", "no_next_recorded": "sin siguiente paso registrado", "no_open_work_items": "Nada en curso", diff --git a/website/src/i18n/locales/fr.json b/website/src/i18n/locales/fr.json index a607f8e9fce..a095fdcba3a 100644 --- a/website/src/i18n/locales/fr.json +++ b/website/src/i18n/locales/fr.json @@ -2398,6 +2398,7 @@ "kind_reply": "Répondu", "kind_review": "Revue", "kind_skip": "Écarté", + "kind_sweep": "Parcouru", "kind_yield": "Libéré", "no_next_recorded": "aucune étape suivante enregistrée", "no_open_work_items": "Rien en cours", diff --git a/website/src/i18n/locales/hi.json b/website/src/i18n/locales/hi.json index 6316ac35aeb..c2e481e2f40 100644 --- a/website/src/i18n/locales/hi.json +++ b/website/src/i18n/locales/hi.json @@ -2517,6 +2517,7 @@ "kind_reply": "जवाब दिया", "kind_review": "समीक्षा", "kind_skip": "दायरे से बाहर", + "kind_sweep": "जाँचा गया", "kind_yield": "छोड़ दिया", "no_next_recorded": "अगला क़दम दर्ज नहीं", "no_open_work_items": "कुछ भी चालू नहीं", diff --git a/website/src/i18n/locales/it.json b/website/src/i18n/locales/it.json index c859519cc40..ec76742980b 100644 --- a/website/src/i18n/locales/it.json +++ b/website/src/i18n/locales/it.json @@ -2354,6 +2354,7 @@ "kind_reply": "Risposto", "kind_review": "Revisione", "kind_skip": "Scartata", + "kind_sweep": "Verificato", "kind_yield": "Rilasciata", "no_next_recorded": "nessun passo successivo registrato", "no_open_work_items": "Niente in corso", diff --git a/website/src/i18n/locales/ja.json b/website/src/i18n/locales/ja.json index 943aeff1eb5..1d7cb97b410 100644 --- a/website/src/i18n/locales/ja.json +++ b/website/src/i18n/locales/ja.json @@ -2304,6 +2304,7 @@ "kind_reply": "返信済み", "kind_review": "レビュー", "kind_skip": "対象外", + "kind_sweep": "確認済み", "kind_yield": "取り下げ", "no_next_recorded": "次の手順は記録されていません", "no_open_work_items": "進行中の作業はありません", diff --git a/website/src/i18n/locales/ko.json b/website/src/i18n/locales/ko.json index 6a78286fc94..018bc4613eb 100644 --- a/website/src/i18n/locales/ko.json +++ b/website/src/i18n/locales/ko.json @@ -2298,6 +2298,7 @@ "kind_reply": "답변함", "kind_review": "리뷰", "kind_skip": "범위 외 판단", + "kind_sweep": "확인함", "kind_yield": "양보함", "no_next_recorded": "기록된 다음 단계 없음", "no_open_work_items": "진행 중인 작업 없음", diff --git a/website/src/i18n/locales/pt.json b/website/src/i18n/locales/pt.json index 8e20f19ff07..a8b6b68580f 100644 --- a/website/src/i18n/locales/pt.json +++ b/website/src/i18n/locales/pt.json @@ -2354,6 +2354,7 @@ "kind_reply": "Respondido", "kind_review": "Revisão", "kind_skip": "Dispensado", + "kind_sweep": "Verificado", "kind_yield": "Liberado", "no_next_recorded": "nenhum próximo passo registrado", "no_open_work_items": "Nada em andamento", diff --git a/website/src/i18n/locales/ru.json b/website/src/i18n/locales/ru.json index fe0dad650a3..668d86bf28c 100644 --- a/website/src/i18n/locales/ru.json +++ b/website/src/i18n/locales/ru.json @@ -2404,6 +2404,7 @@ "kind_reply": "Отвечено", "kind_review": "Ревью", "kind_skip": "Отклонено", + "kind_sweep": "Проверено", "kind_yield": "Отпущено", "no_next_recorded": "следующий шаг не записан", "no_open_work_items": "Ничего в работе", diff --git a/website/src/i18n/locales/zh-CN.json b/website/src/i18n/locales/zh-CN.json index c41df2135b0..e0ea6f5b8ce 100644 --- a/website/src/i18n/locales/zh-CN.json +++ b/website/src/i18n/locales/zh-CN.json @@ -2254,6 +2254,7 @@ "kind_reply": "已回复", "kind_review": "评审", "kind_skip": "超范围略过", + "kind_sweep": "已巡检", "kind_yield": "已让出", "no_next_recorded": "未记录下一步", "no_open_work_items": "没有进行中的工作", diff --git a/website/src/test/IssueRadarCrewPage.test.tsx b/website/src/test/IssueRadarCrewPage.test.tsx index e98e4409f7e..717147b029e 100644 --- a/website/src/test/IssueRadarCrewPage.test.tsx +++ b/website/src/test/IssueRadarCrewPage.test.tsx @@ -287,6 +287,82 @@ describe('CrewPageView — work log 24h boundary', () => { await screen.findByTestId('work-log-row-e-1h') expect(screen.queryByTestId('work-log-earlier')).not.toBeInTheDocument() }) + + it('renders a crew-level line with an em dash rather than a bare hash', async () => { + // A `sweep` line records that the crew checked the queue and took nothing, so + // it belongs to no issue and the backend omits `number` entirely. The Issue + // column is monospaced and right-aligned against real numbers, so a bare `#` + // would read as a number that failed to load. + const sweep = event('e-sweep', 1, { + kind: 'sweep', + text: 'checked 42 open issues, took none', + number: undefined, + }) + api.crew.mockResolvedValue(payload({ events: [sweep, event('e-ci', 2)] })) + renderPage() + + const row = await screen.findByTestId('work-log-row-e-sweep') + const issueCell = within(row).getAllByRole('cell')[1] + expect(issueCell.textContent).toBe('\u2014') + expect(issueCell.textContent).not.toContain('#') + // The numbered sibling is untouched, so the guard is on absence, not on kind. + const numbered = within(screen.getByTestId('work-log-row-e-ci')).getAllByRole('cell')[1] + expect(numbered.textContent).toBe('#2251') + }) + + it('translates the sweep kind through the catalog like every other kind', async () => { + const sweep = event('e-sweep', 1, { kind: 'sweep', text: 'queue empty', number: undefined }) + api.crew.mockResolvedValue(payload({ events: [sweep] })) + renderPage() + + const row = await screen.findByTestId('work-log-row-e-sweep') + expect(within(row).getByText(copy('kind_sweep'))).toBeInTheDocument() + }) + + it('renders a sweep timestamp as a past instant, making no claim about now', async () => { + // The WHEN column states when the line was written and nothing more. An + // earlier revision qualified the newest sweep as "checking since ...", which + // reads as a present-tense activity claim -- and nothing here evidences that a + // crew is alive: a crash or a lost timer leaves `enabled` true, so the row + // would have asserted ongoing checking for a crew that had stopped, masking + // the one failure mode this view exists to help an operator notice. A bare + // timestamp cannot make that claim, so a sweep reads like every other kind. + const sweep = event('e-sweep', 1, { kind: 'sweep', text: 'queue empty', number: undefined }) + api.crew.mockResolvedValue(payload({ events: [sweep, event('e-ci', 3)] })) + renderPage() + + const when = within(await screen.findByTestId('work-log-row-e-sweep')).getAllByRole('cell')[0] + // The relative string comes from the real formatter, not a hardcoded literal: + // its wording is locale data, so pinning "1 hour ago" would assert the + // formatter's current output rather than this row's own rendering. + const rendered = (await import('../i18n/format')).fmtRelative(ago(1), { now: NOW.getTime() }) + expect(when.textContent).toBe(rendered) + }) + + it('renders a sweep the same whether the crew is running or paused', async () => { + // The rendering cannot depend on liveness, because the page has no evidence of + // it. Asserting both states pins that: a future qualifier gated on `!paused` + // would fix only the operator-pause mode and still misreport a crashed crew. + const sweep = event('e-sweep', 2, { kind: 'sweep', text: 'queue empty', number: undefined }) + const expected = (await import('../i18n/format')).fmtRelative(ago(2), { now: NOW.getTime() }) + + api.crew.mockResolvedValue(payload({ events: [sweep] })) + const running = renderPage() + const whenRunning = within( + await screen.findByTestId('work-log-row-e-sweep'), + ).getAllByRole('cell')[0] + expect(whenRunning.textContent).toBe(expected) + running.unmount() + + api.crew.mockResolvedValue( + payload({ crew: crew({ enabled: false, paused_reason: 'operator paused' }), events: [sweep] }), + ) + renderPage() + const whenPaused = within( + await screen.findByTestId('work-log-row-e-sweep'), + ).getAllByRole('cell')[0] + expect(whenPaused.textContent).toBe(expected) + }) }) describe('CrewPageView — the next column', () => {