diff --git a/README.md b/README.md index 28a82b89..51ffb32f 100644 --- a/README.md +++ b/README.md @@ -38,14 +38,14 @@ its `container` status, and its repo and slug: ══════════════════════════════════════════════════════════════════════════ panopticon 6 tasks ────────────────────────────────────────────────────────────────────────── - state turn container repo slug[memo] - ITERATING agent live web-api add-oauth[Add OAuth login] - PLANNING user live web-api fix-upload[Flaky S3 upload] - MERGING agent starting dashboard dark-mode[Dark-mode theme] - ITERATING user ⚠ down web-api migrate-db[Move to Postgres] - ORCHESTRATING agent live infra q3-cleanup[Q3 tech-debt] - PLANNING agent live infra └─ drop-py38[Drop Python 3.8] - COMPLETE agent – web-api ship-readme[README refresh] + state turn container repo ❏ ➚ slug[memo] + ITERATING agent live web-api ❏ ➚ add-oauth[Add OAuth login] + PLANNING user live web-api ❏ fix-upload[Flaky S3 upload] + MERGING agent starting dashboard ➚ dark-mode[Dark-mode theme] + ITERATING user ⚠ down web-api ❏ ➚ migrate-db[Move to Postgres] + ORCHESTRATING agent live infra ❏ q3-cleanup[Q3 tech-debt] + PLANNING agent live infra └─ drop-py38[Drop Python 3.8] + COMPLETE agent – web-api ❏ ➚ ship-readme[README refresh] ────────────────────────────────────────────────────────────────────────── t attach n new task x drop / search d detail ? help q quit ══════════════════════════════════════════════════════════════════════════ @@ -55,7 +55,9 @@ The `turn` column is color-coded live: green when the agent is working, yellow w move, and red (`⚠`) when a task is blocked waiting on you, so you can tell at a glance which agents need you. The `container` column tracks each agent's sandbox as it spawns (`queued → … → live`, or `down` when one needs a respawn), and governed sub-tasks nest under their governor (`└─`). -Press `t` to drop into any task's session, `?` for the full key list. +The `❏ ➚` column marks what a task carries — `❏` an artifact to read (`a` lists them), `➚` a link +such as its PR (`p` opens it). Press `t` to drop into any task's session, `?` for the full key +list. ## Requirements diff --git a/src/panopticon/core/artifacts.py b/src/panopticon/core/artifacts.py index 6bf74124..33c7628a 100644 --- a/src/panopticon/core/artifacts.py +++ b/src/panopticon/core/artifacts.py @@ -46,6 +46,16 @@ def validate_segment(segment: str) -> None: raise InvalidArtifactName(f"invalid artifact segment: {segment!r}") +def is_hidden(name: str) -> bool: + """Whether an artifact is hidden from the operator's default view. + + Dotfile artifacts are agent bookkeeping — cross-turn state like ``.babysit-ci-state.json`` — + rather than documents a human asked for. The dashboard hides them behind a "Show hidden" + toggle and the task list's artifact mark ignores them, so the rule lives here rather than + being spelled out at each surface.""" + return name.startswith(".") + + def mcp_uri(task_id: str, name: str) -> str: """The canonical MCP resource URI for an artifact (the shared resolver). @@ -79,6 +89,16 @@ async def get(self, task_id: str, name: str) -> bytes | None: async def list(self, task_id: str) -> list[str]: """Return the names of a task's artifacts (empty if none).""" + async def has_unhidden_artifacts(self, task_id: str) -> bool: + """Whether the task has at least one artifact the operator would want to open. + + The task list renders a mark per row from this, so it answers the question without + materialising names. Concrete, not abstract: the default is written in terms of + :meth:`list`, which every adapter must provide, so one that has no cheaper way to tell + still inherits a correct implementation (the filesystem store overrides it with a + directory scan that stops at the first hit).""" + return any(not is_hidden(name) for name in await self.list(task_id)) + async def link_slug(self, task_id: str, slug: str) -> None: """Expose a task's artifacts under a readable ``slug`` alias (best-effort). diff --git a/src/panopticon/taskservice/api.py b/src/panopticon/taskservice/api.py index 6ba2704d..ce8ca3f9 100644 --- a/src/panopticon/taskservice/api.py +++ b/src/panopticon/taskservice/api.py @@ -91,6 +91,11 @@ class TaskSummaryOut(BaseModel): sort_weight: int = 0 depends_on_task_ids: list[str] = [] provisioned: bool + #: Whether the task has at least one unhidden artifact — the dashboard's artifact mark. + #: Computed like ``container_status`` (artifacts are files, not a task column), attached on + #: serialization by ``_task_summary_out``. Summary-only: the single-task shapes don't carry + #: it, since ``GET /tasks/{id}/artifacts`` already answers it exactly for one task. + has_artifacts: bool = False container_status: str = "–" lifecycle_detail: str | None = None runner_host: str | None = ( @@ -398,9 +403,13 @@ def _task_out(task: Task) -> TaskOut: out.runner_host = service.runner_host(task.claimed_by) return out - def _task_summary_out(task: Task) -> TaskSummaryOut: - """Serialize a task to the cheap summary shape (no history), with computed status fields.""" + def _task_summary_out(task: Task, *, has_artifacts: bool = False) -> TaskSummaryOut: + """Serialize a task to the cheap summary shape (no history), with computed status fields. + + ``has_artifacts`` is passed in rather than read here: it lives in the artifact store, + not on the task, so resolving it needs an await this synchronous serializer can't do.""" out = TaskSummaryOut.model_validate(task) + out.has_artifacts = has_artifacts out.container_status = service.container_status(task).value lifecycle = service.lifecycle(task.id) out.lifecycle_detail = lifecycle.detail if lifecycle is not None else None @@ -557,7 +566,10 @@ async def list_tasks( # Read version and snapshot in a single thread call so no event-loop yield can # interleave a mutation between them — preserving the original atomicity invariant. version, tasks_raw = await service._tasks_snapshot(terminal=terminal) - tasks = [_task_summary_out(t) for t in tasks_raw] + tasks = [ + _task_summary_out(t, has_artifacts=await service.has_unhidden_artifacts(t.id)) + for t in tasks_raw + ] response.headers[TASKS_VERSION_HEADER] = str(version) return tasks diff --git a/src/panopticon/taskservice/artifacts_fs.py b/src/panopticon/taskservice/artifacts_fs.py index df234f2c..1ed5ed08 100644 --- a/src/panopticon/taskservice/artifacts_fs.py +++ b/src/panopticon/taskservice/artifacts_fs.py @@ -9,9 +9,15 @@ from __future__ import annotations import asyncio +import os from pathlib import Path -from panopticon.core.artifacts import ArtifactStore, InvalidArtifactName, validate_segment +from panopticon.core.artifacts import ( + ArtifactStore, + InvalidArtifactName, + is_hidden, + validate_segment, +) class FilesystemArtifactStore(ArtifactStore): @@ -53,6 +59,30 @@ async def list(self, task_id: str) -> list[str]: lambda: sorted(p.name for p in task_dir.iterdir() if p.is_file()) ) + def _has_artifacts_sync(self, task_id: str) -> bool: + """Whether the task's directory holds at least one unhidden file. + + ``os.scandir`` rather than ``Path.iterdir``: it stops at the first hit (the caller wants + a boolean, not a listing), its entries answer ``is_file()`` from the data the scan already + returned instead of a ``stat`` apiece, and it raises for a missing directory *here* rather + than lazily on iteration — ``Path.iterdir`` is a generator before 3.13, so the error would + escape this ``try`` on the Python versions we still support. + """ + try: + with os.scandir(self._task_dir(task_id)) as entries: + return any(not is_hidden(entry.name) and entry.is_file() for entry in entries) + except (FileNotFoundError, NotADirectoryError): + return False + + async def has_unhidden_artifacts(self, task_id: str) -> bool: + """Scan the task's directory directly rather than going through :meth:`list`. + + The inherited default builds and sorts the full name list to answer a question the first + unhidden entry settles. The task list asks this for every visible task on every refresh, + so the shortcut is worth the override. + """ + return await asyncio.to_thread(self._has_artifacts_sync, task_id) + def _link_slug_sync(self, task_id: str, slug: str) -> None: validate_segment(task_id) validate_segment(slug) diff --git a/src/panopticon/taskservice/service.py b/src/panopticon/taskservice/service.py index 5649718a..9fba5a53 100644 --- a/src/panopticon/taskservice/service.py +++ b/src/panopticon/taskservice/service.py @@ -739,6 +739,11 @@ async def record_provisioning(self, task_id: str, *, branch: str, clone: str) -> async def put_artifact(self, task_id: str, name: str, content: bytes) -> None: await self.get_task(task_id) # ensure the task exists await self._artifacts.put(task_id, name, content) + # Artifacts live outside the store, so writing one bumps no version of its own — but the + # task list reports whether a task *has* one, so a parked long-poll has to wake or the + # first plan.md would go unnoticed until some unrelated mutation. Same treatment as the + # other ephemeral (non-stored) changes. + self._notify_change() _log.debug("task %s: artifact %s written", task_id, name) async def get_artifact(self, task_id: str, name: str) -> bytes | None: @@ -749,6 +754,15 @@ async def list_artifacts(self, task_id: str) -> list[str]: await self.get_task(task_id) return await self._artifacts.list(task_id) + async def has_unhidden_artifacts(self, task_id: str) -> bool: + """Whether the task has an artifact worth marking in the task list. + + No ``get_task`` guard (unlike the readers above): this is a display predicate asked of + tasks the caller has already read, once per row, and a task with no artifacts and a task + that doesn't exist both answer ``False``. Paying for a store read per row to tell those + apart would buy nothing.""" + return await self._artifacts.has_unhidden_artifacts(task_id) + # -- liveness ----------------------------------------------------------------- # # Liveness is connection-scoped: a container holds the ``/live`` stream open for its whole diff --git a/src/panopticon/terminal/dashboard.py b/src/panopticon/terminal/dashboard.py index fa740695..237a4775 100644 --- a/src/panopticon/terminal/dashboard.py +++ b/src/panopticon/terminal/dashboard.py @@ -41,6 +41,11 @@ Arrow keys skip the ensemble row (it is not a real task). Expanding or collapsing does not affect the task service — it is pure display state local to the dashboard. +The `❏ ➚` **marks column** (left of `slug[memo]`, its header doubling as the legend) flags what a +task carries: `❏` when it has at least one unhidden artifact (`a` lists them — dotfile artifacts +are agent bookkeeping and don't count) and `➚` when it has a `url` (`p` opens it). Each mark keeps +its own slot, so they read as two vertical rails rather than shifting per row. + The `container` column shows each task's container status: `live` (an active registration), `down` (was up, container gone — respawn with `R`), `starting` (claimed, no registration yet — its container is still coming up), `healing` (the runner is self-healing an orphan), or `–` (unclaimed @@ -103,7 +108,7 @@ from textual.worker import get_current_worker from panopticon.client import JsonObj, TaskServiceClient -from panopticon.core.artifacts import InvalidArtifactName, validate_segment +from panopticon.core.artifacts import InvalidArtifactName, is_hidden, validate_segment from panopticon.core.dirs import ARTIFACTS_DIR from panopticon.core.models import resolve_agent_cli from panopticon.core.state import TERMINAL_LABELS @@ -187,6 +192,32 @@ def _dim(cell: Text | str) -> Text: return t +# The task-list marks column: one glyph per slot, separated by a space, so each mark reads down +# its own vertical line (slot 1 artifact, slot 2 link) and a row missing one doesn't shift the +# other. +# +# Both glyphs are East_Asian_Width=Neutral with no emoji presentation form, i.e. exactly one cell +# in every terminal. That is the whole reason for these two rather than 📁/🔗: emoji are +# Width=Wide (two cells, and fonts disagree on the details), which would make the column's width +# depend on the viewer. Keep any replacement in the same class — `⚠`/`✓` elsewhere in this file +# are the other examples. +_ARTIFACT_MARK = "❏" # U+274F, has at least one unhidden artifact (`a` lists them) +_LINK_MARK = "➚" # U+279A, has a url (`p` opens it) +_MARKS_LABEL = f"{_ARTIFACT_MARK} {_LINK_MARK}" # the header doubles as the legend +_MARKS_HEADER = Text(_MARKS_LABEL) + + +def _marks_cell(task: JsonObj) -> Text: + """The marks column: ``❏`` when the task has an unhidden artifact, ``➚`` when it has a url. + + Always three cells wide (mark, gap, mark) — an absent mark renders as a space rather than + collapsing — so the two marks stay in their own columns and the cell width can't vary by + row.""" + artifact = _ARTIFACT_MARK if task.get("has_artifacts") else " " + link = _LINK_MARK if task.get("url") else " " + return Text(f"{artifact} {link}", style="dim") + + def _slug_cell(task: JsonObj, prefix: str = "") -> Text: """The ``slug[memo]`` column: the slug followed by the task's memo in brackets. @@ -1721,22 +1752,18 @@ class ArtifactScreen(_OptionListModal[tuple[str, str]]): def __init__(self, title: str, all_names: list[str]) -> None: self._all_names = all_names - visible = [n for n in all_names if not n.startswith(".")] + visible = [n for n in all_names if not is_hidden(n)] super().__init__(title, visible) def _extra_widgets(self) -> Iterable[Widget]: yield Label( "enter: open · e: open local file · ctrl+a: attach · esc: cancel", id="artifact-hint" ) - if any(n.startswith(".") for n in self._all_names): + if any(is_hidden(n) for n in self._all_names): yield SpaceCheckbox("Show hidden", id="show-hidden") def on_checkbox_changed(self, event: Checkbox.Changed) -> None: - names = ( - self._all_names - if event.value - else [n for n in self._all_names if not n.startswith(".")] - ) + names = self._all_names if event.value else [n for n in self._all_names if not is_hidden(n)] option_list = self.query_one(OptionList) option_list.clear_options() for name in names: @@ -1906,11 +1933,16 @@ def action_close(self) -> None: def _setup_task_columns(table: DataTable[Any], *, multi_runner: bool) -> None: - """Add the task table's columns. Includes a "runner" column when tasks span multiple hosts.""" + """Add the task table's columns. Includes a "runner" column when tasks span multiple hosts. + + The marks column sits immediately left of the name it annotates, and last among the + fixed-width columns so the variable-width ``slug[memo]`` stays rightmost.""" if multi_runner: - table.add_columns("state", "turn", "container", "runner", "repo", Text("slug[memo]")) + table.add_columns( + "state", "turn", "container", "runner", "repo", _MARKS_HEADER, Text("slug[memo]") + ) else: - table.add_columns("state", "turn", "container", "repo", Text("slug[memo]")) + table.add_columns("state", "turn", "container", "repo", _MARKS_HEADER, Text("slug[memo]")) class Dashboard(App[None]): @@ -2140,6 +2172,7 @@ def _add_row(task: JsonObj, prefix: str) -> None: Text(""), *runner_blank, Text(""), + Text(""), # marks: a placeholder stands for hidden rows, so it carries none slug_cell, key=f"{_ENSEMBLE_KEY_PREFIX}{gov_id}", ) @@ -2151,6 +2184,7 @@ def _add_row(task: JsonObj, prefix: str) -> None: Text(task.get("runner_host") or "") if self._multi_runner else None ) repo_cell: Text | str = _repo_cell(task, self._repo_names) + marks_cell = _marks_cell(task) slug_cell_real = _slug_cell(task, prefix) if task["state"] in TERMINAL_LABELS: state_cell = _dim(state_cell) @@ -2159,6 +2193,7 @@ def _add_row(task: JsonObj, prefix: str) -> None: if runner_cell is not None: runner_cell = _dim(runner_cell) repo_cell = _dim(repo_cell) + marks_cell = _dim(marks_cell) slug_cell_real = _dim(slug_cell_real) elif _snooze_label(task, display_now) is not None: # An active snooze mutes the whole row (the turn cell already carries the label). @@ -2168,6 +2203,7 @@ def _add_row(task: JsonObj, prefix: str) -> None: if runner_cell is not None: runner_cell = _dim(runner_cell) repo_cell = _dim(repo_cell) + marks_cell = _dim(marks_cell) slug_cell_real = _dim(slug_cell_real) runner_extra = (runner_cell,) if runner_cell is not None else () table.add_row( @@ -2176,6 +2212,7 @@ def _add_row(task: JsonObj, prefix: str) -> None: status_cell, *runner_extra, repo_cell, + marks_cell, slug_cell_real, key=task["id"], ) diff --git a/tests/taskservice/test_api.py b/tests/taskservice/test_api.py index a2fb0a19..cc03f3ab 100644 --- a/tests/taskservice/test_api.py +++ b/tests/taskservice/test_api.py @@ -550,3 +550,26 @@ def test_report_unknown_responsibility(gated_client: TestClient) -> None: f"/tasks/{task_id}/responsibilities", json={"key": "ghost", "status": "met"} ) assert resp.status_code == 400 + + +def test_task_list_reports_whether_a_task_has_unhidden_artifacts(client: TestClient) -> None: + # The dashboard renders a mark per row, so presence has to ride along on the list response + # rather than costing a request per task. Dotfile artifacts are agent bookkeeping: they + # don't count as something the operator can open. + def make() -> str: + task_id: str = client.post("/tasks", json={"repo_id": "r1", "workflow": "spike"}).json()[ + "id" + ] + return task_id + + visible, hidden_only, bare = make(), make(), make() + assert client.put(f"/tasks/{visible}/artifacts/plan.md", content=b"# Plan").status_code == 204 + assert ( + client.put( + f"/tasks/{hidden_only}/artifacts/.babysit-ci-state.json", content=b"{}" + ).status_code + == 204 + ) + + marks = {t["id"]: t["has_artifacts"] for t in client.get("/tasks").json()} + assert marks == {visible: True, hidden_only: False, bare: False} diff --git a/tests/taskservice/test_artifacts.py b/tests/taskservice/test_artifacts.py index 526a738b..3a4e1065 100644 --- a/tests/taskservice/test_artifacts.py +++ b/tests/taskservice/test_artifacts.py @@ -7,7 +7,13 @@ import pytest -from panopticon.core.artifacts import InvalidArtifactName, decode_segment, mcp_uri +from panopticon.core.artifacts import ( + ArtifactStore, + InvalidArtifactName, + decode_segment, + is_hidden, + mcp_uri, +) from panopticon.taskservice.artifacts_fs import FilesystemArtifactStore @@ -152,3 +158,47 @@ def test_decode_segment_reverses_mcp_uri_encoding() -> None: for name in ("plan.md", "my notes.md", "a+b&c.md", ".hidden"): encoded = mcp_uri("t1", name).rsplit("/", 1)[1] assert decode_segment(encoded) == name + + +def test_is_hidden_is_the_dotfile_rule() -> None: + # One definition, shared by the dashboard's "show hidden" toggle and the task list's mark. + assert is_hidden(".babysit-ci-state.json") + assert not is_hidden("plan.md") + assert not is_hidden("notes.tar.gz") + + +def test_has_unhidden_artifacts_ignores_hidden_and_missing(tmp_path: Path) -> None: + store = FilesystemArtifactStore(tmp_path) + asyncio.run(store.put("visible", "plan.md", b"# Plan")) + asyncio.run(store.put("mixed", ".babysit-ci-state.json", b"{}")) + asyncio.run(store.put("mixed", "notes.md", b"notes")) + asyncio.run(store.put("hidden-only", ".babysit-ci-state.json", b"{}")) + assert asyncio.run(store.has_unhidden_artifacts("visible")) + assert asyncio.run(store.has_unhidden_artifacts("mixed")) # hidden siblings don't mask it + assert not asyncio.run(store.has_unhidden_artifacts("hidden-only")) + # "absent" was never written at all — no directory on disk, and that's not an error. + assert not asyncio.run(store.has_unhidden_artifacts("absent")) + + +def test_has_unhidden_artifacts_default_works_without_the_override() -> None: + # An adapter with no cheaper way to answer inherits a correct implementation from the ABC: + # the default is written in terms of list(), which every store must provide. + class InMemoryStore(ArtifactStore): + def __init__(self) -> None: + self.files: dict[str, dict[str, bytes]] = {} + + async def put(self, task_id: str, name: str, content: bytes) -> None: + self.files.setdefault(task_id, {})[name] = content + + async def get(self, task_id: str, name: str) -> bytes | None: + return self.files.get(task_id, {}).get(name) + + async def list(self, task_id: str) -> list[str]: + return sorted(self.files.get(task_id, {})) + + store = InMemoryStore() + asyncio.run(store.put("visible", "plan.md", b"# Plan")) + asyncio.run(store.put("hidden-only", ".state.json", b"{}")) + assert asyncio.run(store.has_unhidden_artifacts("visible")) + assert not asyncio.run(store.has_unhidden_artifacts("hidden-only")) + assert not asyncio.run(store.has_unhidden_artifacts("absent")) diff --git a/tests/taskservice/test_change_feed.py b/tests/taskservice/test_change_feed.py index 6b38dd72..cd7ae465 100644 --- a/tests/taskservice/test_change_feed.py +++ b/tests/taskservice/test_change_feed.py @@ -90,3 +90,23 @@ async def test_quiet_wait_times_out_without_changing_the_version(tmp_path: Path) resp = await http.get("/tasks", params={"wait": 0.1, "since": version}) assert resp.status_code == 200 assert int(resp.headers[TASKS_VERSION_HEADER]) == version + + +async def test_writing_an_artifact_wakes_a_parked_long_poll(tmp_path: Path) -> None: + # Artifacts are files, not store rows, so writing one bumps no stored version of its own — + # but the listing reports has_artifacts, so the write has to wake the feed or the mark + # wouldn't appear until some unrelated mutation happened to come along. + svc = await _service(tmp_path) + task = await svc.create_task("r1", "spike") + async with _client(svc) as http: + version = int((await http.get("/tasks")).headers[TASKS_VERSION_HEADER]) + + waiter = asyncio.ensure_future(http.get("/tasks", params={"wait": 5, "since": version})) + await asyncio.sleep(0.05) + assert not waiter.done() + + await svc.put_artifact(task.id, "plan.md", b"# Plan") + + resp = await asyncio.wait_for(waiter, timeout=1) + assert int(resp.headers[TASKS_VERSION_HEADER]) > version + assert resp.json()[0]["has_artifacts"] is True diff --git a/tests/terminal/test_dashboard.py b/tests/terminal/test_dashboard.py index 8b841749..4893f1c1 100644 --- a/tests/terminal/test_dashboard.py +++ b/tests/terminal/test_dashboard.py @@ -19,8 +19,11 @@ from panopticon.terminal import dashboard from panopticon.terminal.dashboard import ( + _ARTIFACT_MARK, _ENSEMBLE_KEY_PREFIX, _INDEFINITE_SNOOZE_UNTIL, + _LINK_MARK, + _MARKS_LABEL, _SNOOZE_DURATION, Dashboard, SpaceCheckbox, @@ -29,6 +32,7 @@ _group_by_governor, _group_section, _make_sort_key, + _marks_cell, _matches, _repo_cell, _slug_cell, @@ -65,6 +69,21 @@ } +def _col_labels(table: DataTable) -> list[str]: + return [str(c.label) for c in table.columns.values()] + + +def _col_index(table: DataTable, label: str) -> int: + """A column's position, by header label. Row cells are read positionally, so resolving the + index here keeps a newly inserted column from renumbering assertions all over this file.""" + return _col_labels(table).index(label) + + +def _slug_of(table: DataTable, row_key: str) -> Any: + """The row's ``slug[memo]`` cell — the one most assertions here reach for.""" + return table.get_row(row_key)[_col_index(table, "slug[memo]")] + + def _raise(*args: Any, **kwargs: Any) -> Any: """Stand in for a failing REST call (e.g. a down service).""" raise RuntimeError("service unavailable") @@ -490,7 +509,7 @@ async def test_terminal_tasks_are_faded() -> None: assert keys == ["t-a", "t-b", "t-done", "t-drop"] # active before terminal, no separator # Active rows: slug cell has no dim span. for task_id in ("t-a", "t-b"): - slug_cell = table.get_row(task_id)[4] + slug_cell = _slug_of(table, task_id) assert not any(s.style == "dim" for s in slug_cell._spans) # Terminal rows: every cell carries dim styling. for task_id in ("t-done", "t-drop"): @@ -509,7 +528,7 @@ async def test_active_only_rows_not_faded() -> None: keys = [str(k.value) for k in table.rows] assert keys == ["t-a", "t-b"] for task_id in ("t-a", "t-b"): - slug_cell = table.get_row(task_id)[4] + slug_cell = _slug_of(table, task_id) assert not any(s.style == "dim" for s in slug_cell._spans) @@ -619,7 +638,7 @@ async def test_expired_snooze_resumes_normal_presentation_without_mutating() -> table = app.query_one("#tasks", DataTable) row = table.get_row("task-abcdef0123") assert row[1].plain == "agent" and row[1].style == "green" # ordinary turn derivation - slug_cell = row[4] + slug_cell = _slug_of(table, "task-abcdef0123") assert not any(s.style == "dim" for s in slug_cell._spans) # not muted # Expiry is display-only: the dashboard never wrote the stored fact. assert client.snoozed == [] @@ -1269,6 +1288,35 @@ def test_status_cell_displays_the_composed_status_color_coded() -> None: assert _status_cell({}).plain == "–" # missing → em-dash, no crash +def test_marks_cell_flags_artifacts_and_links_in_fixed_slots() -> None: + # Slot 1 is the artifact mark, slot 2 the link mark; an absent mark leaves its slot blank so + # the other one doesn't slide over. + assert _marks_cell({"has_artifacts": True, "url": "https://pr"}).plain == _MARKS_LABEL + assert _marks_cell({"has_artifacts": True}).plain == f"{_ARTIFACT_MARK} " + assert _marks_cell({"url": "https://pr"}).plain == f" {_LINK_MARK}" + assert _marks_cell({}).plain == " " # neither + assert _marks_cell({"has_artifacts": False, "url": None}).plain == " " + assert _marks_cell({}).style == "dim" # annotation, not competing with the name + + +def test_marks_cell_is_the_same_width_whatever_it_carries() -> None: + # The column only stays aligned while every combination measures the same. This is the guard + # against swapping in an East_Asian_Width=Wide glyph (an emoji) later: cell_len would jump to + # 4 or 5 for the marked rows and the column would render ragged. + from rich.cells import cell_len + + widths = { + cell_len(_marks_cell(task).plain) + for task in ( + {"has_artifacts": True, "url": "https://pr"}, + {"has_artifacts": True}, + {"url": "https://pr"}, + {}, + ) + } + assert widths == {3} + + async def test_task_counter_shows_agent_versus_active_counts() -> None: # Counter shows agent-turn active / total active; terminal tasks are excluded. # pause() lets Footer's _bindings_ready recompose fire so #task-counter is mounted; @@ -2928,10 +2976,8 @@ async def test_governed_task_appears_under_governor_in_dashboard() -> None: await pilot.pause() order = [str(k.value) for k in table.rows] assert order == ["gov", "wrk"] - gov_row = table.get_row("gov") - wrk_row = table.get_row("wrk") - assert gov_row[4].plain == "orchestrator" # slug column (index 4) — no prefix - assert wrk_row[4].plain == "└─ worker" # last (only) child gets └─ + assert _slug_of(table, "gov").plain == "orchestrator" # no prefix + assert _slug_of(table, "wrk").plain == "└─ worker" # last (only) child gets └─ async def test_active_governor_keeps_terminal_child_in_active_section() -> None: @@ -2974,9 +3020,9 @@ async def test_active_governor_keeps_terminal_child_in_active_section() -> None: assert keys.index("gov") < keys.index("done") assert keys.index("wrk") < keys.index("done") # Active governor is not faded; both terminal tasks (standalone and governed) are. - assert not any(s.style == "dim" for s in table.get_row("gov")[4]._spans) + assert not any(s.style == "dim" for s in _slug_of(table, "gov")._spans) for task_id in ("wrk", "done"): - slug = table.get_row(task_id)[4] + slug = _slug_of(table, task_id) assert slug._spans and all(s.style == "dim" for s in slug._spans), ( f"{task_id} slug should be dim" ) @@ -3067,8 +3113,7 @@ async def test_enter_on_governor_collapses_to_ensemble_row() -> None: assert "wrk" not in keys assert f"{_ENSEMBLE_KEY_PREFIX}gov" in keys # The ensemble row's slug cell reads "..." (dim, checked by plain text). - ens_row = table.get_row(f"{_ENSEMBLE_KEY_PREFIX}gov") - assert ens_row[4].plain == "└─ ..." + assert _slug_of(table, f"{_ENSEMBLE_KEY_PREFIX}gov").plain == "└─ ..." async def test_enter_again_on_governor_expands_ensemble() -> None: @@ -3183,11 +3228,61 @@ async def test_search_shows_all_ancestors_when_deep_child_matches() -> None: assert set(keys) == {"root", "mid", "leaf"} # whole chain visible -# -- multi-runner column ----------------------------------------------------------- +# -- marks column ------------------------------------------------------------------ -def _col_labels(table: DataTable) -> list[str]: - return [str(c.label) for c in table.columns.values()] +async def test_marks_column_sits_left_of_the_name_in_both_layouts() -> None: + # Present whether or not the runner column is, and always immediately left of slug[memo] — + # the marks annotate the name, and the variable-width name column stays rightmost. + header = _MARKS_LABEL + single = _FakeClient([{**_TASK, "id": "t-a"}], runners=[{"id": "r1", "host": "host-a"}]) + multi = _FakeClient( + [{**_TASK, "id": "t-a", "runner_host": "host-a"}], + runners=[{"id": "r1", "host": "host-a"}, {"id": "r2", "host": "host-b"}], + ) + for client in (single, multi): + app = Dashboard(client) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + labels = _col_labels(app.query_one("#tasks", DataTable)) + assert labels.index(header) == labels.index("slug[memo]") - 1 + + +async def test_marks_column_reflects_artifacts_and_url() -> None: + tasks = [ + {**_TASK, "id": "t-both", "has_artifacts": True, "url": "https://pr/1"}, + {**_TASK, "id": "t-artifact", "has_artifacts": True, "url": None}, + {**_TASK, "id": "t-link", "has_artifacts": False, "url": "https://pr/2"}, + {**_TASK, "id": "t-bare", "has_artifacts": False, "url": None}, + ] + app = Dashboard(_FakeClient(tasks)) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + table = app.query_one("#tasks", DataTable) + idx = _col_index(table, _MARKS_LABEL) + assert table.get_row("t-both")[idx].plain == _MARKS_LABEL + assert table.get_row("t-artifact")[idx].plain == f"{_ARTIFACT_MARK} " + assert table.get_row("t-link")[idx].plain == f" {_LINK_MARK}" + assert table.get_row("t-bare")[idx].plain == " " + + +async def test_ensemble_placeholder_row_spans_every_column() -> None: + # The synthetic collapsed-ensemble row stands in for hidden tasks, so it carries no marks — + # but it still needs a cell per column or the table misaligns. + governor = {**_TASK, "id": "gov", "slug": "orchestrator", "has_artifacts": True} + governed = {**_TASK, "id": "wrk", "slug": "worker", "governor_task_id": "gov"} + app = Dashboard(_FakeClient([governor, governed])) # type: ignore[arg-type] + async with app.run_test() as pilot: + await pilot.pause() + table = app.query_one("#tasks", DataTable) + ens_row = table.get_row(f"{_ENSEMBLE_KEY_PREFIX}gov") # governors start collapsed + assert len(ens_row) == len(table.columns) + assert ens_row[_col_index(table, _MARKS_LABEL)].plain == "" + # The governor itself still shows its own marks. + assert table.get_row("gov")[_col_index(table, _MARKS_LABEL)].plain == f"{_ARTIFACT_MARK} " + + +# -- multi-runner column ----------------------------------------------------------- async def test_runner_column_absent_for_single_runner() -> None: