From eabe90b0cfc0b6243b738a7c65f340c28e2c5e6f Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Wed, 12 Aug 2026 18:18:00 +0800 Subject: [PATCH 1/3] feat(dashboard): show what a session is waiting for, not just that it is (DRC-4015) Knowing a session is blocked is worth less than knowing what on. "Force push to main?" is a decision a person can make from the board. "open question (AskUserQuestion)" is a reason to go and look, which is the trip the row exists to save. Two changes, because the board note's "render what is already parsed" was half right and the wrong half is where the work was. The parse threw the text away. A pending input tool was kept as {name, ts}, and the question lives in the tool_use block's `input`, which was dropped. It is now summarised at parse time rather than kept raw: `ExitPlanMode` carries a whole plan, sometimes thousands of words, and a plan has no business in a session row or in the caches a row is built from. A plan becomes its first line, which is its own title in practice; a question becomes the first question with a count of the rest. Both bounded, both scrubbed, because a transcript is a file Cargento does not write and this text lands in the DOM. An agreeing overlay then blanked it anyway. No overlay constructor sets `detail`, so every needs-input patch carried None and applying it wrote that over whatever the collector had found. On a default install, with the bundled hook present, that happened every time. The collector's detail now survives, narrowly: only when the row was already Needs input and stays Needs input, which is the case where the overlay agrees about the state and contradicts nothing. Working and Idle still clear the field. A working detail such as `running Bash` is true of a running session and false of one stopped at a gate, so it must not follow the row into a wait, and an answered question must not outlive the overlay that retired it, which is DRC-4095 and DRC-4097. The text still comes from the transcript alone, so it is there when the record reached disk and absent when it did not (N-4). The row says a question is open either way and can only sometimes say which. SKILL.md says so rather than smoothing it over: a field that is usually present teaches a reader to trust it and then fails silently on the sessions that matter. Rationale in docs/design-needs-input.md (N-7). Co-Authored-By: Claude Opus 5 Signed-off-by: Kent Huang --- cargento/skills/cargento/SKILL.md | 4 +- .../cargento/cargento_runtime/aggregate.py | 23 ++++- .../cargento/cargento_runtime/claude_data.py | 47 +++++++++- .../cargento_runtime/collectors/claude.py | 16 +++- .../cargento/cargento_runtime/config.py | 5 ++ cargento/skills/cargento/tests/test_claude.py | 88 +++++++++++++++++++ .../skills/cargento/tests/test_observation.py | 67 ++++++++++++++ docs/design-needs-input.md | 39 +++++++- 8 files changed, 281 insertions(+), 8 deletions(-) diff --git a/cargento/skills/cargento/SKILL.md b/cargento/skills/cargento/SKILL.md index 6f46d79..7f9ce7a 100644 --- a/cargento/skills/cargento/SKILL.md +++ b/cargento/skills/cargento/SKILL.md @@ -11,7 +11,7 @@ Cargento is an agnostic agent cartography and visualization tool: a local web da Store locations are resolved per platform, and the documented relocation variables are honored: `CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `GEMINI_CLI_HOME` (the CLI creates `.gemini` inside it; relocates the Antigravity store with it), `COPILOT_HOME`, `PI_CODING_AGENT_DIR`, and `PI_CODING_AGENT_SESSION_DIR`. When one is set it is authoritative — no fallback to the default location. Run `--diagnose` to see every path searched. Data sources (read-only, no external calls; all parsing is defensive — a broken harness store is skipped, never fatal): -- `~/.claude/projects/*/.jsonl` — Claude transcript tails: session discovery, titles, token usage, pending AskUserQuestion detection +- `~/.claude/projects/*/.jsonl` — Claude transcript tails: session discovery, titles, token usage, pending AskUserQuestion detection and the text of the question it is asking - Claude subagents, two generations: modern harnesses write each subagent as a top-level `~/.claude/projects/*/.jsonl` whose records carry `agentName` + `teamName: "session-"` — these fold into the parent session (named pill, freshness, output rate) and never appear as standalone sessions; legacy `/subagents/agent-*.jsonl` + `.meta.json` files are still recognized (fresh mtime = running), including the `subagents/workflows//` directory a workflow fan-out nests its agents in. A bare `agentName` from a top-level `--agent` launch has no parent relation and remains a standalone session; `agentSetting`, not `agentName`, supplies any Spacedock role. Agent writes count as parent activity in their own right, so a session parked on a long background workflow reads Working rather than Idle - Spacedock workflows: a Claude session launched by Spacedock carries an `agentSetting` of `spacedock:first-officer` or `spacedock:ensign` in its first transcript records, which is how the role badge appears. A first officer also records its `spacedock status --boot` output — counted only when it arrives as command output, never as ordinary conversation text — which names each workflow directory and each entity-state directory absolutely. The ordered stage list comes from the workflow `README.md` frontmatter, and each entity's current stage from the `status` in its own state-file frontmatter; boot's `dispatchable` list is only a snapshot of what was ready to move at boot, so it fills in behind the state directory rather than standing in for it. Those two kinds of frontmatter are the only project files Cargento reads — see the repository's SECURITY.md for the contract, and `--no-spacedock` to disable it - `~/.claude/tasks//N.json` — tracked task state (subject, status, activeForm); current bare-UUID and older `session-` directories are supported @@ -140,7 +140,7 @@ Both layers notify on the *transition* into needs-input, not on every refresh a **Known gap:** idle nudges (`idle_prompt`) pop without marking the session blocked. The server delivers those on macOS, but the page only notifies on a needs-input transition — so on Linux and Windows an idle nudge produces no popup today. Closing it needs a one-shot event channel in `/api/data`; it is tracked alongside the native Linux and Windows backends rather than bolted on here. -1. **Transcript detection** — an open `AskUserQuestion` or `ExitPlanMode` flips the session to Needs input on the next collection, *when the record has reached disk*. Claude Code buffers it and may not write it until the gate is answered, so treat this as an opportunistic early signal rather than a source to rely on (an open dashboard tab is what drives collections, so keep one open). +1. **Transcript detection** — an open `AskUserQuestion` or `ExitPlanMode` flips the session to Needs input on the next collection, *when the record has reached disk*. Claude Code buffers it and may not write it until the gate is answered, so treat this as an opportunistic early signal rather than a source to rely on (an open dashboard tab is what drives collections, so keep one open). When the record is there, the row shows the question itself, or a plan's first line, rather than the tool's name; when it is not, the row still says a question is open but cannot say which. Both readings are normal for the same session. 2. **Lifecycle hooks** — `Notification` and `SessionEnd` hooks in user settings (`~/.claude/settings.json`) POSTing their payloads to `http://127.0.0.1:4553/api/notify`. Notifications cover permission prompts and idle waits, even with no browser tab open. The structured `notification_type` decides whether a notification is actionable. Idle nudges (`idle_prompt`, message "Claude is waiting for your input") pop once but never mark the session blocked; authentication, completion and computer-use status notifications do neither; permission prompts, MCP elicitation dialogs and a worker's permission or network request create Needs-input state. A type not on either list is treated as actionable, so a notification kind added upstream surfaces rather than disappearing. `SessionEnd` clears a standing hook when Claude exits cleanly. These hooks are NOT installed by the plugin — if the user wants path 2, offer to add them to their `~/.claude/settings.json`: Use the bundled `notify_hook.py` (next to `server.py`) rather than a `curl` one-liner. The one-liner is POSIX-only end to end — single-quoting, `/dev/null`, `|| true`, and `--data-binary @-` all fail in `cmd.exe`, and Windows PowerShell 5.1 aliases `curl` to `Invoke-WebRequest` and has no `||`. One interpreter invocation behaves the same in every shell, exits 0 even when the dashboard is not running, and refuses to POST anywhere but loopback. diff --git a/cargento/skills/cargento/cargento_runtime/aggregate.py b/cargento/skills/cargento/cargento_runtime/aggregate.py index ca96679..580cb06 100644 --- a/cargento/skills/cargento/cargento_runtime/aggregate.py +++ b/cargento/skills/cargento/cargento_runtime/aggregate.py @@ -22,6 +22,27 @@ from .state import RuntimeState +def _keep_wait_detail(session: Session, patch: Mapping[str, Any]) -> Mapping[str, Any]: + """The patch, minus a `state_detail` that would blank a standing wait. + + No overlay constructor sets `detail`, so every needs-input patch carries + None, and applying it erased whatever the collector had found. For a Claude + row that is the open question itself, which is the one thing a person + stopped at a gate wants to read. + + Narrow on purpose. It applies only when the row was already Needs input and + stays Needs input, because the overlay is then agreeing about the state and + disagreeing about nothing. Working and Idle must keep clearing the field, or + a working detail such as `running Bash` follows the row into a wait, and a + question that has been answered outlives the overlay that retired it. + """ + if session.get("state") != "needs_input" or patch.get("state") != "needs_input": + return patch + if patch.get("state_detail") is not None or not session.get("state_detail"): + return patch + return {key: value for key, value in patch.items() if key != "state_detail"} + + class OverlaySource(Protocol): """The narrow view of the coordinator that a collection needs. @@ -358,7 +379,7 @@ def _apply_overlays(self, out_sessions: list[Session], *, now: float) -> None: activity_grace_sec=self.config.overlay_wait_activity_grace_sec, ) self._note_dispute(session, patch, overlays, now=now) - runtime_events.apply_patch(session, patch) + runtime_events.apply_patch(session, _keep_wait_detail(session, patch)) else: # No ledger for this row means nothing can be disagreeing with it. self._clear_dispute(harness, sid) diff --git a/cargento/skills/cargento/cargento_runtime/claude_data.py b/cargento/skills/cargento/cargento_runtime/claude_data.py index d44e819..6f5a51f 100644 --- a/cargento/skills/cargento/cargento_runtime/claude_data.py +++ b/cargento/skills/cargento/cargento_runtime/claude_data.py @@ -19,12 +19,49 @@ from cargento_runtime import state as runtime_state if TYPE_CHECKING: + from collections.abc import Mapping + from cargento_runtime.config import RuntimeConfig from cargento_runtime.state import RuntimeState # Tools that mean Claude is blocked on the human, not just running long. INPUT_TOOLS = {"AskUserQuestion", "ExitPlanMode"} + +def input_summary(block: Mapping[str, Any], *, limit: int) -> str: + """What an input tool is actually asking, in one bounded line, or "". + + Summarised at parse time rather than kept raw, so the question is the only + thing that survives the read. `ExitPlanMode` carries a whole plan, which can + run to thousands of words and has no business sitting in a session row or in + the caches one is built from. + + The two shapes are Claude Code's, read off 160 real records rather than a + page: `AskUserQuestion` carries `questions`, a list of objects with a + `question`, and `ExitPlanMode` carries `plan`. Both are treated as untrusted, + because a transcript is a file on disk that Cargento does not write. + + A plan is reduced to its first line, which is the plan's own title in + practice. Anything longer is a document, and a row is not where a document + goes. + """ + payload = records.as_dict(block.get("input")) + if block.get("name") == "ExitPlanMode": + # Split before scrubbing, not after: `safe_text` turns a newline into a + # space, so a plan scrubbed first has no line to take the first of. + first_line = str(payload.get("plan") or "").split("\n", 1)[0] + return records.safe_text(first_line, limit).strip().lstrip("#").strip()[:limit] + questions = [ + text + for item in records.as_list(payload.get("questions")) + if (text := records.safe_text(records.as_dict(item).get("question"), limit).strip()) + ] + if not questions: + return "" + extra = len(questions) - 1 + return f"{questions[0]} (+{extra} more)"[:limit] if extra else questions[0][:limit] + + # Claude's own sentinel for an assistant record it generated locally, without # the request ever reaching the API: a cancellation notice, an error banner, a # tool-limit message. It is not a model, and it is common — one top-level @@ -180,7 +217,7 @@ def analyze_transcript(config: RuntimeConfig, state: RuntimeState, path: str) -> "model": None, "model_sidechain": None, "usage_events": [], # (epoch, output_tokens) - "pending_input_tool": None, # {"name", "ts"} awaiting the human + "pending_input_tool": None, # {"name", "ts", "asks"} awaiting the human "last_tool": None, "last_event_ts": 0, # Newest record the *agent* wrote, which is not the same as the newest @@ -190,7 +227,7 @@ def analyze_transcript(config: RuntimeConfig, state: RuntimeState, path: str) -> "last_assistant_ts": 0, "last_user_event": last_user_event(config, state, path), } - pending: dict[Any, Any] = {} # tool_use id -> {"name", "ts"} for INPUT_TOOLS only + pending: dict[Any, Any] = {} # tool_use id -> {"name", "ts", "asks"} for INPUT_TOOLS only for line in runtime_io.read_tail(config, path): if not line or line[0] != "{": continue @@ -221,7 +258,11 @@ def analyze_transcript(config: RuntimeConfig, state: RuntimeState, path: str) -> if isinstance(c, dict) and c.get("type") == "tool_use": info["last_tool"] = c.get("name") if c.get("name") in INPUT_TOOLS: - pending[c.get("id")] = {"name": c.get("name"), "ts": ep} + pending[c.get("id")] = { + "name": c.get("name"), + "ts": ep, + "asks": input_summary(c, limit=config.input_summary_cap_chars), + } elif t == "user": for c in records.as_list(records.message_dict(d).get("content")): if isinstance(c, dict) and c.get("type") == "tool_result": diff --git a/cargento/skills/cargento/cargento_runtime/collectors/claude.py b/cargento/skills/cargento/cargento_runtime/collectors/claude.py index 153936d..d75fa31 100644 --- a/cargento/skills/cargento/cargento_runtime/collectors/claude.py +++ b/cargento/skills/cargento/cargento_runtime/collectors/claude.py @@ -437,7 +437,21 @@ def collect( p = info["pending_input_tool"] session_state = "needs_input" blocked_since = p["ts"] or last_activity - state_detail = f"open question ({p['name']}), waiting {runtime_sessions.fmt_duration(runtime_sessions.age(config, now, p['ts'])) if p['ts'] else '?'}" + waited = ( + runtime_sessions.fmt_duration(runtime_sessions.age(config, now, p["ts"])) + if p["ts"] + else "?" + ) + # The question itself when the record carried it, the tool's name when + # it did not. Both happen: the record reaches disk on no schedule, so + # this reads as one or the other rather than appearing and vanishing + # for the same session. See docs/design-needs-input.md (N-4). + asks = p.get("asks") or "" + state_detail = ( + f"{asks}, waiting {waited}" + if asks + else f"open question ({p['name']}), waiting {waited}" + ) # Fresh activity in the session's *own* transcript still beats a hook: # Claude Code emits "waiting for your input" notifications for sessions # that keep running via background tasks and will resume on their own. diff --git a/cargento/skills/cargento/cargento_runtime/config.py b/cargento/skills/cargento/cargento_runtime/config.py index 698301d..bb9213a 100644 --- a/cargento/skills/cargento/cargento_runtime/config.py +++ b/cargento/skills/cargento/cargento_runtime/config.py @@ -122,6 +122,10 @@ class RuntimeConfig: prompt_path_collapse_min_length: int first_line_json_cap_bytes: int notification_body_cap_bytes: int + # What an open question may occupy on a row. Characters, not bytes: this one + # is measured where it is read, and a plan's first line is prose rather than + # a payload. + input_summary_cap_chars: int # The quota fetch (SECURITY.md, "Usage quota reads"): the contract's # five-minute floor between requests to one vendor, the request timeout, # and the read caps on the credential file and the response body. @@ -375,6 +379,7 @@ def build_runtime_config( prompt_path_collapse_min_length=25, first_line_json_cap_bytes=200_000, notification_body_cap_bytes=65_536, + input_summary_cap_chars=160, usage_poll_floor_sec=300, usage_fetch_timeout_sec=10, usage_credentials_cap_bytes=65_536, diff --git a/cargento/skills/cargento/tests/test_claude.py b/cargento/skills/cargento/tests/test_claude.py index 1200047..09deba3 100644 --- a/cargento/skills/cargento/tests/test_claude.py +++ b/cargento/skills/cargento/tests/test_claude.py @@ -1770,3 +1770,91 @@ def test_a_bookkeeping_write_does_not_move_own_activity(self) -> None: parent = next(s for s in sessions if s["sid"].startswith("12345678")) self.assertAlmostEqual(now - 600, parent["own_activity"], delta=1.0) + + +class InputSummaryTest(unittest.TestCase): + """What an open gate is asking, reduced to one bounded line. DRC-4015.""" + + @staticmethod + def _block(name: str, payload: Any) -> dict[str, Any]: + return {"type": "tool_use", "id": "q-1", "name": name, "input": payload} + + def test_a_question_is_summarised_as_the_question(self) -> None: + summary = claude_data.input_summary( + self._block( + "AskUserQuestion", + {"questions": [{"header": "Auth", "question": "Which auth method?"}]}, + ), + limit=160, + ) + self.assertEqual("Which auth method?", summary) + + def test_several_questions_name_the_first_and_count_the_rest(self) -> None: + summary = claude_data.input_summary( + self._block( + "AskUserQuestion", + {"questions": [{"question": "Which one?"}, {"question": "And then?"}]}, + ), + limit=160, + ) + self.assertEqual("Which one? (+1 more)", summary) + + def test_a_plan_is_reduced_to_its_first_line(self) -> None: + # Option B of three: a plan runs to thousands of words, and its first + # line is its title in practice. A row is not where a document goes. + plan = "# Rewrite the collector\n\nStep one, do the thing.\n" + ("x" * 5_000) + summary = claude_data.input_summary(self._block("ExitPlanMode", {"plan": plan}), limit=160) + self.assertEqual("Rewrite the collector", summary) + + def test_every_summary_is_capped(self) -> None: + long_question = "y" * 400 + for block in ( + self._block("AskUserQuestion", {"questions": [{"question": long_question}]}), + self._block("ExitPlanMode", {"plan": long_question}), + ): + self.assertEqual(40, len(claude_data.input_summary(block, limit=40))) + + def test_control_characters_do_not_reach_a_row(self) -> None: + # A transcript is a file Cargento does not write, and this text lands in + # the DOM. + summary = claude_data.input_summary( + self._block("AskUserQuestion", {"questions": [{"question": "we\x00ird\nthing"}]}), + limit=160, + ) + self.assertNotIn("\x00", summary) + self.assertNotIn("\n", summary) + + def test_a_shape_that_is_not_there_summarises_to_nothing(self) -> None: + # The record reaches disk on no schedule and its shape is Claude Code's + # rather than something this repo pins, so absent has to be ordinary. + payloads: list[dict[str, Any]] = [ + {}, + {"questions": []}, + {"questions": "not a list"}, + {"plan": None}, + ] + for payload in payloads: + for name in ("AskUserQuestion", "ExitPlanMode"): + self.assertEqual( + "", claude_data.input_summary(self._block(name, payload), limit=160) + ) + + def test_a_pending_question_carries_its_text_through_the_transcript_read(self) -> None: + record = { + "type": "assistant", + "timestamp": "2026-01-01T00:00:00+00:00", + "message": { + "role": "assistant", + "content": [ + self._block( + "AskUserQuestion", {"questions": [{"question": "Force push to main?"}]} + ) + ], + }, + } + with tempfile.TemporaryDirectory() as tmp: + transcript = Path(tmp) / "session.jsonl" + transcript.write_text(json.dumps(record) + "\n") + config, state = runtime() + info = claude_data.analyze_transcript(config, state, str(transcript)) + self.assertEqual("Force push to main?", info["pending_input_tool"]["asks"]) diff --git a/cargento/skills/cargento/tests/test_observation.py b/cargento/skills/cargento/tests/test_observation.py index be5f262..941676d 100644 --- a/cargento/skills/cargento/tests/test_observation.py +++ b/cargento/skills/cargento/tests/test_observation.py @@ -732,6 +732,73 @@ def signalling(*, show_all: bool) -> Any: coordinator.stop(timeout=5) +class WaitDetailTest(unittest.TestCase): + """An agreeing overlay must not blank the question the collector found.""" + + def setUp(self) -> None: + self.state = support.reset_runtime() + + class Source: + def __init__(self, kind: str) -> None: + self.kind = kind + + def overlays_for(self, harness: str, sid: str) -> list[events.Overlay]: + del harness, sid + return [ + events.Overlay( + harness="claude", + sid=PREFIX, + arrival_seq=1, + kind=self.kind, + at=support.SERVER_STARTED, + ) + ] + + def note_rows(self, keys: set[tuple[str, str]]) -> None: + pass + + def drop_counters(self) -> dict[str, int]: + return {} + + def _apply(self, kind: str, collected: str, detail: str | None) -> dict[str, Any]: + app = support.build_app() + app.overlays = self.Source(kind) + session: dict[str, Any] = { + "harness": "claude", + "sid": PREFIX, + "state": collected, + "state_detail": detail, + } + app._apply_overlays([session], now=support.SERVER_STARTED) + return session + + def test_an_agreeing_wait_overlay_leaves_the_question_on_the_row(self) -> None: + # No overlay constructor sets `detail`, so the patch always carried None + # and applying it blanked the one thing a person at a gate wants to read. + session = self._apply(events.OVERLAY_NEEDS_INPUT, "needs_input", "Force push to main?") + self.assertEqual("needs_input", session["state"]) + self.assertEqual("Force push to main?", session["state_detail"]) + self.assertEqual("event", session["acquisition"], "the rest of the patch still applied") + + def test_a_working_overlay_still_clears_it(self) -> None: + # Otherwise a question that has been answered outlives the overlay that + # retired the wait, which is DRC-4095 and DRC-4097 territory. + session = self._apply(events.OVERLAY_WORKING, "needs_input", "Force push to main?") + self.assertEqual("working", session["state"]) + self.assertIsNone(session["state_detail"]) + + def test_a_wait_overlay_over_a_working_row_does_not_carry_its_detail_in(self) -> None: + # `running Bash` is true of a session that is working and false of one + # stopped at a gate, so it must not follow the row into the wait. + session = self._apply(events.OVERLAY_NEEDS_INPUT, "working", "running Bash") + self.assertEqual("needs_input", session["state"]) + self.assertIsNone(session["state_detail"]) + + def test_a_row_with_no_detail_of_its_own_is_unchanged(self) -> None: + session = self._apply(events.OVERLAY_NEEDS_INPUT, "needs_input", None) + self.assertIsNone(session["state_detail"]) + + class StateDisputeTest(unittest.TestCase): """When an overlay overrules a collector that had found a wait. DRC-4139.""" diff --git a/docs/design-needs-input.md b/docs/design-needs-input.md index e007b00..e8ca070 100644 --- a/docs/design-needs-input.md +++ b/docs/design-needs-input.md @@ -330,7 +330,44 @@ Half a number is still the half that was missing, and it is the half this milest Deciding first and measuring afterwards is how DRC-4134 got a mechanism that turned out to be impossible. -## What is still not measured +## N-7: saying what it is waiting for, and why that is two changes rather than one + +Knowing a session is blocked is worth less than knowing what it is blocked on. "Force push to main?" +is a decision a person can make from the board; "open question (AskUserQuestion)" is a reason to go +and look, which is the trip the row exists to save (DRC-4015). + +The board note that opened that issue said this was pure rendering, over data already parsed. Half of +that was right and the half that was wrong is where the work is. + +**The parse threw the text away.** A pending input tool was kept as `{name, ts}`, and the question +lives in the `tool_use` block's `input`, which was dropped on the floor. Summarising it at parse time +rather than keeping it raw is deliberate: `ExitPlanMode` carries a whole plan, sometimes thousands of +words, and a plan has no business in a session row or in the caches a row is built from. A plan is +reduced to its first line, which is its own title in practice. A question is the first question, with +a count of the rest. + +**An agreeing overlay then blanked it anyway.** No overlay constructor sets `detail`, so every +needs-input patch carried None and `apply_patch` wrote that over whatever the collector had found. +On a default install, where the bundled hook is present, that erasure happened every time. The fix +is narrow on purpose: the collector's detail survives only when the row was already Needs input and +stays Needs input, which is the case where the overlay agrees about the state and contradicts +nothing. + +Working and Idle still clear the field, and that is not an oversight. A working detail such as +`running Bash` is true of a session that is running and false of one stopped at a gate, so it must +not follow the row into a wait. And a question that has been answered must not outlive the overlay +that retired the wait, which is exactly DRC-4095 and DRC-4097. + +### What it deliberately does not do + +The text still comes from the transcript alone, so it appears when the record has reached disk and +does not when it has not (N-4). The row says a question is open either way; it can only sometimes say +which. That is stated in `SKILL.md` rather than smoothed over, because a field that is usually there +teaches a reader to trust it and then fails silently on the sessions that matter. + +Sourcing the text from the event path instead would make it reliable and is not a rendering change: +the envelope drops the tool name and the tool input at the hook, deliberately, and `SECURITY.md` +states that as a property. Reopening it is a security decision, not this one. `notification_type` is present on every Notification payload, and its value list was read from the emit sites in an installed 2.1.226 bundle rather than observed on the wire. No capture in From d91d52961b44b95d443b6129efbddcb93ca3d7a2 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Wed, 12 Aug 2026 18:38:07 +0800 Subject: [PATCH 2/3] docs(needs-input): name the second absence window, and stop claiming what is unmeasured Two review findings, both about the writing outrunning the evidence rather than about the code. N-7 documented one reason the question can be missing, the record not having reached disk, and missed the ordinary one. For up to `overlay_working_ttl_sec` after a turn starts, a live working overlay overrules the collector outright: the row reads Working, the detail is cleared, and the question is invisible even though the parse has it. `_keep_wait_detail` cannot help, because it only holds when the patch agrees the row is waiting. N-7's own argument about a field that is usually there applies to that window word for word, so it now names it, and points at N-6's recorder as the thing that counts it. SKILL.md gains the same caveat. "On a default install the erasure happened every time" was asserted. It needs an `input_requested` overlay, which only `PermissionRequest` mints, and `captures/` holds no Claude `PermissionRequest` record: the only capture of that name is Codex's, and it is a negative result. Whether the hook fires for `AskUserQuestion` and `ExitPlanMode` is an adapter claim this repository requires a capture for, so the paragraph now says the erasure is reachable and its frequency unknown, and names what would settle it. Four smaller things from the same pass: `safe_text` stripped C0 and DEL and passed the zero-width, bidi-embedding and isolate ranges. Those reorder how the text after them renders, so a record could make a row read as something it does not say. Shared function, so this covers every untrusted string on a row, not only the new one. A plan now summarises to its first *usable* line. A plan opening with a blank line, a fence or a bullet gave "```, waiting 2m", which is a worse row than the tool's name it replaced. The seam between the parse and the row had no test: `InputSummaryTest` stopped at `analyze_transcript` and `WaitDetailTest` started at hand-built rows. Two collector-level tests now pin both readings, the question present and absent. SECURITY.md's rationale for keeping `state_detail` out of dispute records said it can carry a permission prompt's text. That is now also an open question's and a plan's first line, which makes the exclusion better justified rather than different. Co-Authored-By: Claude Opus 5 Signed-off-by: Kent Huang --- SECURITY.md | 2 +- cargento/skills/cargento/SKILL.md | 2 +- .../cargento/cargento_runtime/claude_data.py | 9 ++- .../cargento/cargento_runtime/records.py | 12 +++- cargento/skills/cargento/tests/test_claude.py | 66 +++++++++++++++++++ docs/design-needs-input.md | 24 +++++-- 6 files changed, 106 insertions(+), 9 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index f20f344..8c82b6c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -192,7 +192,7 @@ navigations, and answers 503 when the process runs without a coordinator. The same route serves the bounded record of state disputes, where an event overruled a session the dashboard had read as waiting. A record holds the same fields plus the two activity timestamps the reducer compared, and no more: the row's title and its state detail are deliberately absent, because -a state detail can carry a permission prompt's own text. +a state detail can carry a permission prompt's own text, an open question's, or a plan's first line. ## Known and accepted diff --git a/cargento/skills/cargento/SKILL.md b/cargento/skills/cargento/SKILL.md index 7f9ce7a..c1cbeda 100644 --- a/cargento/skills/cargento/SKILL.md +++ b/cargento/skills/cargento/SKILL.md @@ -140,7 +140,7 @@ Both layers notify on the *transition* into needs-input, not on every refresh a **Known gap:** idle nudges (`idle_prompt`) pop without marking the session blocked. The server delivers those on macOS, but the page only notifies on a needs-input transition — so on Linux and Windows an idle nudge produces no popup today. Closing it needs a one-shot event channel in `/api/data`; it is tracked alongside the native Linux and Windows backends rather than bolted on here. -1. **Transcript detection** — an open `AskUserQuestion` or `ExitPlanMode` flips the session to Needs input on the next collection, *when the record has reached disk*. Claude Code buffers it and may not write it until the gate is answered, so treat this as an opportunistic early signal rather than a source to rely on (an open dashboard tab is what drives collections, so keep one open). When the record is there, the row shows the question itself, or a plan's first line, rather than the tool's name; when it is not, the row still says a question is open but cannot say which. Both readings are normal for the same session. +1. **Transcript detection** — an open `AskUserQuestion` or `ExitPlanMode` flips the session to Needs input on the next collection, *when the record has reached disk*. Claude Code buffers it and may not write it until the gate is answered, so treat this as an opportunistic early signal rather than a source to rely on (an open dashboard tab is what drives collections, so keep one open). When the record is there, the row shows the question itself, or a plan's first line, rather than the tool's name; when it is not, the row still says a question is open but cannot say which. Both readings are normal for the same session. There is also a window of up to 90 seconds after a turn starts where a live event overlay reports Working and the question does not show at all, even though it was parsed. 2. **Lifecycle hooks** — `Notification` and `SessionEnd` hooks in user settings (`~/.claude/settings.json`) POSTing their payloads to `http://127.0.0.1:4553/api/notify`. Notifications cover permission prompts and idle waits, even with no browser tab open. The structured `notification_type` decides whether a notification is actionable. Idle nudges (`idle_prompt`, message "Claude is waiting for your input") pop once but never mark the session blocked; authentication, completion and computer-use status notifications do neither; permission prompts, MCP elicitation dialogs and a worker's permission or network request create Needs-input state. A type not on either list is treated as actionable, so a notification kind added upstream surfaces rather than disappearing. `SessionEnd` clears a standing hook when Claude exits cleanly. These hooks are NOT installed by the plugin — if the user wants path 2, offer to add them to their `~/.claude/settings.json`: Use the bundled `notify_hook.py` (next to `server.py`) rather than a `curl` one-liner. The one-liner is POSIX-only end to end — single-quoting, `/dev/null`, `|| true`, and `--data-binary @-` all fail in `cmd.exe`, and Windows PowerShell 5.1 aliases `curl` to `Invoke-WebRequest` and has no `||`. One interpreter invocation behaves the same in every shell, exits 0 even when the dashboard is not running, and refuses to POST anywhere but loopback. diff --git a/cargento/skills/cargento/cargento_runtime/claude_data.py b/cargento/skills/cargento/cargento_runtime/claude_data.py index 6f5a51f..168a6d2 100644 --- a/cargento/skills/cargento/cargento_runtime/claude_data.py +++ b/cargento/skills/cargento/cargento_runtime/claude_data.py @@ -49,8 +49,13 @@ def input_summary(block: Mapping[str, Any], *, limit: int) -> str: if block.get("name") == "ExitPlanMode": # Split before scrubbing, not after: `safe_text` turns a newline into a # space, so a plan scrubbed first has no line to take the first of. - first_line = str(payload.get("plan") or "").split("\n", 1)[0] - return records.safe_text(first_line, limit).strip().lstrip("#").strip()[:limit] + # The first *usable* line rather than the first line, because a plan can + # open with a blank, a fence or a bullet, and none of those name it. + for raw_line in str(payload.get("plan") or "").split("\n"): + line = records.safe_text(raw_line, limit).strip().lstrip("#*->` ").strip() + if line: + return line[:limit] + return "" questions = [ text for item in records.as_list(payload.get("questions")) diff --git a/cargento/skills/cargento/cargento_runtime/records.py b/cargento/skills/cargento/cargento_runtime/records.py index b4c054d..ddcdc19 100644 --- a/cargento/skills/cargento/cargento_runtime/records.py +++ b/cargento/skills/cargento/cargento_runtime/records.py @@ -8,10 +8,20 @@ from datetime import UTC, datetime from typing import Any +# C0 and DEL, then the zero-width, bidi-embedding and isolate ranges. +_UNSAFE_CHARS = re.compile("[\\x00-\\x1f\\x7f\\u200b-\\u200f\\u202a-\\u202e\\u2066-\\u2069]+") + def safe_text(value: Any, limit: int) -> str: + """Untrusted text, safe to put on a row: no control characters, bounded. + + The bidi and isolate ranges are stripped alongside the C0 set, and not for + tidiness: those characters reorder how the text after them renders, so a + harness record could make a row read as something it does not say. Legitimate + right-to-left text does not need them, since bidi resolves implicitly. + """ text = str(value or "").encode("utf-8", "replace").decode("utf-8") - text = re.sub(r"[\x00-\x1f\x7f]+", " ", text) + text = _UNSAFE_CHARS.sub(" ", text) return text[:limit] diff --git a/cargento/skills/cargento/tests/test_claude.py b/cargento/skills/cargento/tests/test_claude.py index 09deba3..e474df2 100644 --- a/cargento/skills/cargento/tests/test_claude.py +++ b/cargento/skills/cargento/tests/test_claude.py @@ -1806,6 +1806,16 @@ def test_a_plan_is_reduced_to_its_first_line(self) -> None: summary = claude_data.input_summary(self._block("ExitPlanMode", {"plan": plan}), limit=160) self.assertEqual("Rewrite the collector", summary) + def test_a_plan_that_opens_with_furniture_skips_to_the_line_that_names_it(self) -> None: + # A plan can open with a blank line, a fence or a bullet, and none of + # those name it. "```, waiting 2m" is a worse row than the tool's name. + for opening in ("\n\n# Real title\nbody", "```\nReal title\nbody", "- Real title\nbody"): + with self.subTest(opening=opening.split("\n")[0] or "(blank)"): + summary = claude_data.input_summary( + self._block("ExitPlanMode", {"plan": opening}), limit=160 + ) + self.assertEqual("Real title", summary) + def test_every_summary_is_capped(self) -> None: long_question = "y" * 400 for block in ( @@ -1858,3 +1868,59 @@ def test_a_pending_question_carries_its_text_through_the_transcript_read(self) - config, state = runtime() info = claude_data.analyze_transcript(config, state, str(transcript)) self.assertEqual("Force push to main?", info["pending_input_tool"]["asks"]) + + +class QuestionOnTheRowTest(RuntimeTestCase): + """The seam between the parse and the row, which nothing else covers.""" + + def _row(self, tmp: str, payload: Any) -> dict[str, Any]: + projects = Path(tmp) / "projects" + project = projects / "-w-proj" + project.mkdir(parents=True) + transcript = project / "abcdef12-0000-0000-0000-000000000000.jsonl" + transcript.write_text( + json.dumps( + { + "type": "assistant", + "timestamp": datetime.fromtimestamp(time.time() - 120, UTC).isoformat(), + "message": { + "role": "assistant", + "content": [ + { + "type": "tool_use", + "id": "q-1", + "name": "AskUserQuestion", + "input": payload, + } + ], + }, + } + ) + + "\n" + ) + with ( + store_patch(PROJECTS_DIR=str(projects)), + store_patch(TASKS_DIR=str(projects / "tasks")), + ): + sessions = collect()["sessions"] + return next(s for s in sessions if s["sid"].startswith("abcdef12")) + + def test_the_question_reaches_the_row(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + row = self._row(tmp, {"questions": [{"question": "Force push to main?"}]}) + self.assertEqual("needs_input", row["state"]) + self.assertTrue( + row["state_detail"].startswith("Force push to main?, waiting"), + row["state_detail"], + ) + + def test_a_record_carrying_no_question_keeps_the_old_wording(self) -> None: + # The record reaches disk on no schedule and its shape is Claude Code's, + # so absent has to read as ordinary rather than as an empty row. + with tempfile.TemporaryDirectory() as tmp: + row = self._row(tmp, {}) + self.assertEqual("needs_input", row["state"]) + self.assertTrue( + row["state_detail"].startswith("open question (AskUserQuestion), waiting"), + row["state_detail"], + ) diff --git a/docs/design-needs-input.md b/docs/design-needs-input.md index e8ca070..32487ba 100644 --- a/docs/design-needs-input.md +++ b/docs/design-needs-input.md @@ -348,10 +348,18 @@ a count of the rest. **An agreeing overlay then blanked it anyway.** No overlay constructor sets `detail`, so every needs-input patch carried None and `apply_patch` wrote that over whatever the collector had found. -On a default install, where the bundled hook is present, that erasure happened every time. The fix -is narrow on purpose: the collector's detail survives only when the row was already Needs input and -stays Needs input, which is the case where the overlay agrees about the state and contradicts -nothing. +The fix is narrow on purpose: the collector's detail survives only when the row was already Needs +input and stays Needs input, which is the case where the overlay agrees about the state and +contradicts nothing. + +How often that erasure fired is **not measured**, and an earlier draft of this paragraph said it +happened on every default install, which the repository cannot support. It needs an `input_requested` +overlay, which only `PermissionRequest` mints, and `captures/` holds no Claude `PermissionRequest` +record at all: the only capture of that name is Codex's, and it is a negative result. Whether the +hook fires for `AskUserQuestion` and `ExitPlanMode`, as opposed to only for tool-permission prompts, +is an adapter claim of exactly the kind this repository requires a capture for. Until there is one, +the honest statement is that the erasure is reachable and its frequency is unknown. The fix is +correct and costs nothing either way. Working and Idle still clear the field, and that is not an oversight. A working detail such as `running Bash` is true of a session that is running and false of one stopped at a gate, so it must @@ -365,6 +373,14 @@ does not when it has not (N-4). The row says a question is open either way; it c which. That is stated in `SKILL.md` rather than smoothed over, because a field that is usually there teaches a reader to trust it and then fails silently on the sessions that matter. +There is a second window, and it is the ordinary one rather than the exotic one. For up to +`overlay_working_ttl_sec` after a turn starts, a live working overlay overrules the collector +outright: the row reads Working, the detail is cleared, and the question is invisible even when the +record is on disk and the parse has it. `_keep_wait_detail` cannot help there, because it only holds +when the patch agrees the row is waiting, and this patch says Working. That window is bounded, it is +the disagreement N-6's recorder exists to count, and it is the reason the sentence above applies to a +default install and not only to an unlucky flush. + Sourcing the text from the event path instead would make it reliable and is not a rendering change: the envelope drops the tool name and the tool input at the hook, deliberately, and `SECURITY.md` states that as a property. Reopening it is a security decision, not this one. From a0c8bbb10772ceedf0608aab5258a233c25d6cea Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Wed, 12 Aug 2026 18:44:54 +0800 Subject: [PATCH 3/3] fix(records): keep the joiners that words are spelled with The scrub I widened a commit ago took U+200B to U+200F as a range, which sweeps in ZWNJ and ZWJ. ZWNJ is orthographic in Persian and several Indic scripts and sits inside words; ZWJ is what composes an emoji sequence. The substitution replaces with a space, so a Persian title would have been broken mid-word. This is a shared function, so the blast radius was every untrusted string on a row: titles, prompts, model names, not only the question this branch adds. My own check passed because CJK does not use joiners. Neither joiner can reorder text, so keeping them costs no protection. The spoofing vector is the directional marks and the embedding, override and isolate ranges, and those stay covered. The range is now listed character by character across that span with the reason written beside it, since a range is what caused this. Tests both ways: what must be stripped, and what must survive. Escapes rather than literals in the fixture, for the same reason ruff bans literals in source. Co-Authored-By: Claude Opus 5 Signed-off-by: Kent Huang --- .../cargento/cargento_runtime/records.py | 10 +++++-- .../skills/cargento/tests/test_transcripts.py | 26 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cargento/skills/cargento/cargento_runtime/records.py b/cargento/skills/cargento/cargento_runtime/records.py index ddcdc19..ab8dafe 100644 --- a/cargento/skills/cargento/cargento_runtime/records.py +++ b/cargento/skills/cargento/cargento_runtime/records.py @@ -8,8 +8,14 @@ from datetime import UTC, datetime from typing import Any -# C0 and DEL, then the zero-width, bidi-embedding and isolate ranges. -_UNSAFE_CHARS = re.compile("[\\x00-\\x1f\\x7f\\u200b-\\u200f\\u202a-\\u202e\\u2066-\\u2069]+") +# C0 and DEL, the zero-width space, the two directional marks, and the bidi +# embedding and isolate ranges. Listed one by one across U+200B to U+200F rather +# than as a range, because U+200C and U+200D are inside it and must survive: +# ZWNJ is orthographic in Persian and several Indic scripts, sitting inside +# words, and ZWJ is what composes an emoji sequence. Neither can reorder text, +# so keeping them costs no protection, and stripping them would break a title in +# those scripts anywhere in the product, not only on the row that prompted this. +_UNSAFE_CHARS = re.compile("[\\x00-\\x1f\\x7f\\u200b\\u200e\\u200f\\u202a-\\u202e\\u2066-\\u2069]+") def safe_text(value: Any, limit: int) -> str: diff --git a/cargento/skills/cargento/tests/test_transcripts.py b/cargento/skills/cargento/tests/test_transcripts.py index b8e3299..01817c0 100644 --- a/cargento/skills/cargento/tests/test_transcripts.py +++ b/cargento/skills/cargento/tests/test_transcripts.py @@ -832,6 +832,32 @@ def test_safe_text_replaces_controls_and_truncates(self) -> None: self.assertEqual("a b c", records.safe_text("a\x00b\nc", 10)) self.assertEqual("abc", records.safe_text("abcdef", 3)) + def test_safe_text_strips_what_can_reorder_a_row(self) -> None: + # A record that reorders its own rendering can make a row read as + # something it does not say. Escapes rather than literals here for the + # same reason ruff bans them in source: this file has to stay readable. + for name, char in ( + ("zero-width space", "\u200b"), + ("left-to-right mark", "\u200e"), + ("right-to-left mark", "\u200f"), + ("right-to-left override", "\u202e"), + ("first strong isolate", "\u2068"), + ("pop directional isolate", "\u2069"), + ): + with self.subTest(name=name): + self.assertEqual("a b", records.safe_text(f"a{char}b", 10)) + + def test_safe_text_keeps_the_joiners_that_words_are_spelled_with(self) -> None: + # ZWNJ sits inside Persian and Indic words and ZWJ composes an emoji + # sequence. Neither can reorder anything, and stripping them would break + # a title in those scripts anywhere in the product. + for name, text in ( + ("persian zwnj", "\u0645\u06cc\u200c\u062e\u0648\u0627\u0647\u0645"), + ("emoji zwj", "\U0001f468\u200d\U0001f4bb"), + ): + with self.subTest(name=name): + self.assertEqual(text, records.safe_text(text, 50)) + class ReviewFixTest(unittest.TestCase): """Regressions found by the adversarial review passes on PR #7."""