From d261296123e83b25de3b090c33d5dff3e936d6b7 Mon Sep 17 00:00:00 2001 From: Jared Scott Date: Thu, 27 Aug 2026 20:51:07 +0800 Subject: [PATCH 1/3] fix(runtime): narrow the block table, read Codex tool output, re-derive the shape counts DRC-4269, the correctness and comment-accuracy half. - `_derive_block` no longer fires on quoted prose. `not permitted`, `permission denied` and `waiting for your` supplied 4 of the 7 blocks the local Claude corpus publishes and every one sat in a quoted or fenced span; two hit the 200-character cap with the trigger truncated away. They are replaced by first-person forms, and an indicator now has to end on a word boundary so `waiting for you` stops matching the possessive. - `spacedock.tool_result_text` learns Codex's spelling. It read only Claude's and Pi's, so the observer's stage half was structurally unreachable there: 0 of 458 local rollouts yielded a boot envelope, and 10 do now. - The observer's head and tail windows are cut apart on byte offsets, which makes the dedup fallback positional. `payload.id` is absent on 76.4% of the Codex user records inside those windows, so the key fell through to the message text and a repeated prompt kept its oldest position. - A clipped goal keeps its ellipsis: the scrub ran at the cap, not cap + 1. - A leading U+FEFF / U+200B / U+2060 no longer defeats all three `injected_prompt` branches at once. That was the one degenerate class that failed open. - Every count in the prompt-shape comment block is re-derived by `scripts/derive_prompt_shapes.py`, committed here. It prints counts and shape names only, never prompt text, and its guard and test say so. - Three wrong figures in `transcripts.py` corrected against fresh measurement, and the developer-tag counts relabelled as the leading counts they now are. Signed-off-by: Jared Scott --- .github/workflows/quality-gate.yml | 6 +- AGENTS.md | 3 +- .../cargento/cargento_runtime/observer.py | 128 +++++- .../cargento/cargento_runtime/records.py | 139 ++++-- .../cargento/cargento_runtime/spacedock.py | 58 ++- .../cargento/cargento_runtime/transcripts.py | 39 +- .../skills/cargento/tests/test_observer.py | 274 ++++++++++-- .../skills/cargento/tests/test_records.py | 32 ++ .../skills/cargento/tests/test_spacedock.py | 93 ++++ scripts/derive_prompt_shapes.py | 411 ++++++++++++++++++ scripts/tests/test_derive_prompt_shapes.py | 211 +++++++++ 11 files changed, 1290 insertions(+), 104 deletions(-) create mode 100755 scripts/derive_prompt_shapes.py create mode 100644 scripts/tests/test_derive_prompt_shapes.py diff --git a/.github/workflows/quality-gate.yml b/.github/workflows/quality-gate.yml index db9ae094..caa1e8da 100644 --- a/.github/workflows/quality-gate.yml +++ b/.github/workflows/quality-gate.yml @@ -176,7 +176,8 @@ jobs: scripts.tests.test_bump_version \ scripts.tests.test_lint_embedded \ scripts.tests.test_bench_collect \ - scripts.tests.test_capture_hook + scripts.tests.test_capture_hook \ + scripts.tests.test_derive_prompt_shapes coverage report test: @@ -209,7 +210,8 @@ jobs: scripts.tests.test_bump_version \ scripts.tests.test_lint_embedded \ scripts.tests.test_bench_collect \ - scripts.tests.test_capture_hook + scripts.tests.test_capture_hook \ + scripts.tests.test_derive_prompt_shapes coverage report # The threshold lives in pyproject.toml ([tool.coverage.report] diff --git a/AGENTS.md b/AGENTS.md index b87c634f..04eb49b0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -152,7 +152,8 @@ coverage run -m unittest discover -s cargento/skills/cargento/tests -t . coverage run -a -m unittest \ scripts.tests.test_validate_plugins scripts.tests.test_bump_version \ scripts.tests.test_lint_embedded scripts.tests.test_bench_collect \ - scripts.tests.test_capture_hook scripts.tests.test_bench_event_latency + scripts.tests.test_capture_hook scripts.tests.test_bench_event_latency \ + scripts.tests.test_derive_prompt_shapes coverage report # enforces the fail_under threshold from pyproject.toml # Native validators, if the CLIs are installed (they are not available on stock runners): claude plugin validate ./cargento --strict diff --git a/cargento/skills/cargento/cargento_runtime/observer.py b/cargento/skills/cargento/cargento_runtime/observer.py index b29d1a3d..4ab8ee9f 100644 --- a/cargento/skills/cargento/cargento_runtime/observer.py +++ b/cargento/skills/cargento/cargento_runtime/observer.py @@ -66,18 +66,36 @@ # of them hit the 200-character cap with the triggering phrase truncated away, # so the rendered card showed a block whose visible text contained no block # language at all. The first-person forms above already carry the real case. +# +# `not permitted`, `permission denied` and `waiting for your` are the remainder, +# and they go the same way. Re-measured over the whole local Claude corpus (3,774 +# transcripts, 2,828 with an assistant message) the table produced 7 blocks and +# those three supplied 4 of them — every one inside a quoted or fenced span, two +# of them again truncated away by the 200-character cap. They are replaced below +# by the self-state forms that carry the real case; those forms match 0 records +# today, which is the point. An indicator that never fires costs nothing, and +# these three cost a wrong answer each. _BLOCK_INDICATORS = ( "i'm blocked", "i am blocked", "i'm stuck", "i am stuck", "waiting for you", - "waiting for your", "waiting for approval", - "not permitted", - "permission denied", + "i'm waiting for your", + "i am waiting for your", + "i'm not permitted", + "i am not permitted", + "i don't have permission", + "i do not have permission", ) +# What may not follow an indicator. `waiting for you` is a prefix of `waiting for +# your`, so without this the bare phrase would keep matching the possessive the +# line above removes — and the two are not the same claim: "waiting for you." is +# a hand-off, "waiting for your PR to land" is a report about someone else. +_BLOCK_TRAILING_RE = re.compile(r"[A-Za-z0-9]") + class ModelCaller(Protocol): """A cheap model invocation that derives a goal line, or None on failure. @@ -99,7 +117,14 @@ def __call__(self, recent_text: str, entity_stage: str) -> str | None: ... def _is_generic_opener(text: str) -> bool: - """Whether a user message is a generic skill-load directive, not a goal.""" + """Whether a user message is a generic skill-load directive, not a goal. + + One concept with ``records.injected_prompt`` and two disjoint lists: both ask + "is this the harness talking". They are kept apart because this is a *goal* + rule and that is a *record* rule — a generic opener is a real user message + that states nothing, so no other reader should be made to drop it — but a + phrase that belongs on both has to be added to both. + """ stripped = text.strip().lower() return any(stripped.startswith(prefix) for prefix in _GENERIC_OPENER_PREFIXES) @@ -110,6 +135,11 @@ def _is_generic_opener(text: str) -> bool: # shape with no single name to hand the injected-tag lookup, which is exactly # the case `records.injected_prompt` documents: a harness it has no measured # vocabulary for gets the union of every measured set. +# +# So this value is deliberately NOT a key of `records._INJECTED_TAGS`, and the +# dict miss is the mechanism rather than an accident. The Droid half of the name +# is aspirational: `resolve_transcript` answers for claude, codex and pi only, so +# no Droid transcript reaches this module today. _SHARED_MESSAGE_HARNESS = "pi-or-droid" @@ -211,6 +241,11 @@ def _dedup_key(record: dict[str, Any]) -> str: degraded silently to the message text and a prompt repeated verbatim later in the session kept its first, oldest position. Claude spells it ``uuid`` and Codex ``payload.id``; Pi and Droid do spell it ``id``. + + Empty is a real answer and not a failure: ``payload.id`` is absent on 599 of + the 784 Codex user-message records (76.4%) inside this module's own head and + tail windows, measured over the whole local rollout store. The caller's + fallback is positional for that reason — see ``_window_lines``. """ for value in ( record.get("uuid"), @@ -222,12 +257,54 @@ def _dedup_key(record: dict[str, Any]) -> str: return "" +def _window_lines(config: RuntimeConfig, path: str) -> list[str]: + """The head window's lines then the tail window's, with no line in both. + + Cut apart on byte offsets rather than deduped afterwards, because the two + windows overlap on any file under ``observer_head_bytes + tail_bytes`` — + completely, on any file the tail read swallows whole. The old concatenation + leaned on the dedup key to collapse that overlap, and the key falls back to + the message TEXT on the 76.4% of Codex user records carrying no + ``payload.id``: a prompt repeated verbatim later in the session was then + dropped as a duplicate of its own first occurrence, and the goal stayed on + whatever came between them. Disjoint windows make that fallback positional, + so the only thing the key still has to collapse is a genuine replay — a + resumed transcript rewriting earlier records, which carry their original ids. + """ + tail_lines = runtime_io.read_tail(config, path) + try: + size = os.path.getsize(path) + except OSError: + return tail_lines + if size <= config.tail_bytes: + # The tail read took the whole file, so the head window is a prefix of + # what is already here. + return tail_lines + try: + head = runtime_io.read_prefix_bytes(path, max_bytes=config.observer_head_bytes) + except OSError: + return tail_lines + # The first byte the tail read covers. A head line starting at or after it is + # in `tail_lines` already; one starting before it cannot be, because the tail + # read drops its own partial first line. + floor = size - config.tail_bytes + head_lines: list[str] = [] + offset = 0 + for raw in head.split(b"\n"): + if offset >= floor: + break + head_lines.append(raw.decode("utf-8", "replace")) + offset += len(raw) + 1 + return head_lines + tail_lines + + def _extract_messages(config: RuntimeConfig, path: str) -> list[dict[str, str]]: """User and assistant texts from a JSONL transcript, head + tail bounded. The head carries the opening directive; the tail carries the recent window. - Records are deduped by their own id so the overlap region between head and - tail does not double-count, and returned in **record-timestamp** order. + ``_window_lines`` keeps the two disjoint, and records are deduped by their + own id so a resumed transcript's replayed block does not double-count. + Returned in **record-timestamp** order. Ordering by timestamp and not by list position, because list position is not record order. The concatenation itself is fine — the head is read @@ -245,18 +322,12 @@ def _extract_messages(config: RuntimeConfig, path: str) -> list[dict[str, str]]: fires. File order breaks ties, which is what keeps a transcript whose records carry no stamp reading exactly as it did before. """ - try: - head = runtime_io.read_prefix_bytes(path, max_bytes=config.observer_head_bytes) - except OSError: - head = b"" - head_lines = head.decode("utf-8", "replace").split("\n") - tail_lines = runtime_io.read_tail(config, path) ordered: list[tuple[float, int, dict[str, str]]] = [] seen: set[str] = set() # Carried forward so a stampless record sorts beside the stamped one before # it rather than ahead of the whole file. last_ts = 0.0 - for position, raw in enumerate(head_lines + tail_lines): + for position, raw in enumerate(_window_lines(config, path)): if not raw or not raw.lstrip().startswith("{"): continue try: @@ -271,7 +342,7 @@ def _extract_messages(config: RuntimeConfig, path: str) -> list[dict[str, str]]: parsed = _parse_message_record(record) if parsed is None: continue - key = _dedup_key(record) or parsed["text"] + key = _dedup_key(record) or f"#{position}" if key in seen: continue seen.add(key) @@ -333,11 +404,19 @@ def _derive_goal_deterministic( # it reads as `…`, which was 60 of 400 Claude sessions and # 5 of 457 Codex rollouts. `prompt_title` # already owns that rendering (`/review 1287 — with fresh eyes`), and - # strips the wrapper tags off everything else. + # strips the wrapper tags off everything else. It also collapses a long + # absolute path to its basename (`transcripts.shorten_paths`), so a goal + # naming a temp file reads as the file rather than as the path to it. goal = transcripts.prompt_title(config, directives[-1], limit=config.observer_goal_cap_chars) if not goal: return NO_GOAL, None - return records.safe_text(goal, config.observer_goal_cap_chars), None + # Cap plus one, for the reason `records.instruction_line` carries the same + # `+ 1`: `transcripts.clip` appends its ellipsis AFTER cutting to the cap, so + # a clipped goal is cap + 1 characters and a scrub at the cap took the `…` + # straight back off — an unmarked mid-token cut on 8 of the 1,295 goals the + # local Claude corpus publishes (269 of which clip at all). `safe_text` only + # ever shortens, so this cannot lengthen what rendering already bounded. + return records.safe_text(goal, config.observer_goal_cap_chars + 1), None def _derive_stage( @@ -379,6 +458,21 @@ def _derive_stage( return entities[0][1] if entities else "" +def _indicator_hit(lower: str, indicator: str) -> int: + """Where one indicator matches as a whole phrase in lowercased text, or -1. + + Scans past a rejected hit rather than stopping at it: `str.find` returns the + first occurrence, and "waiting for your PR" earlier in a message must not + hide a genuine "waiting for you." later in it. + """ + start = 0 + while (pos := lower.find(indicator, start)) >= 0: + if not _BLOCK_TRAILING_RE.match(lower, pos + len(indicator)): + return pos + start = pos + 1 + return -1 + + def _derive_block( config: RuntimeConfig, messages: list[dict[str, str]], @@ -400,7 +494,7 @@ def _derive_block( text = msg["text"] lower = text.lower() for indicator in _BLOCK_INDICATORS: - pos = lower.find(indicator) + pos = _indicator_hit(lower, indicator) if pos < 0: continue # Extract the sentence around the indicator. diff --git a/cargento/skills/cargento/cargento_runtime/records.py b/cargento/skills/cargento/cargento_runtime/records.py index 274995df..7c4f94e5 100644 --- a/cargento/skills/cargento/cargento_runtime/records.py +++ b/cargento/skills/cargento/cargento_runtime/records.py @@ -601,6 +601,12 @@ def _turn_signal(record: dict[str, Any], harness: str) -> tuple[str, Any] | None isinstance(item, dict) and item.get("type") == "tool_result" for item in content ): return None + # Both names are also in `_CLAUDE_USER_TAGS`, and the duplication is + # deliberate: this is a RECORD rule and that is a TEXT rule. Here the whole + # record is discarded before any turn is counted; there the leading tag + # merely disqualifies the text from standing in for a person's intent. The + # tag set covers seven further names this must NOT refuse, so it cannot be + # read from there — but a name added here belongs in both. if isinstance(content, str) and content.lstrip().startswith( ("", "") ): @@ -617,10 +623,29 @@ def _turn_signal(record: dict[str, Any], harness: str) -> tuple[str, Any] | None # treats those as things a person said reports the wrong goal, the wrong stage, # and the wrong idea of who is waiting on whom. # -# The lists below were derived rather than guessed: 2,737 Codex user-role texts -# across 457 `rollout-*.jsonl` files, and 21,899 Claude user-role texts across -# 3,769 transcripts matching the collector's own glob. Every entry carries its -# measured count, and nothing without one is here. +# The lists below were derived rather than guessed, and +# `scripts/derive_prompt_shapes.py` is the code that derived them — kept, so a +# reviewer can re-run it against their own store rather than take these on +# trust. Every count below is one of its outputs, re-derived 2026-08-27; every +# entry carries one, and nothing without one is here. +# +# **Which records were counted**, because the two halves count different ones: +# +# Codex 458 `rollout-*.jsonl` files. The user set counts LEADING tags over +# the union of two populations — 1,007 `event_msg`/`user_message` +# texts and 1,734 `response_item` message texts with `role: "user"`. +# A union rather than a choice because the record shape moved mid-CLI, +# and it DOUBLE-COUNTS: a build that writes both spells one prompt +# twice, and 974 of the 1,007 `event_msg` prompts have a matching +# `response_item`. So these are occurrences of a shape, not prompts. +# Codex 1,826 `response_item` message texts with `role: "developer"`, for +# the developer set below. +# Claude 211,669 user-role texts across 3,774 transcripts matching the +# collector's own glob. +# +# A live store only grows, so re-running the script produces slightly larger +# figures than these. What has to keep holding is the SHAPE of each claim — which +# tag leads, which never does, which population a count is over — not the digits. # # The two harnesses do not share a vocabulary, which is why there are two sets # rather than one. Codex spells its injections with underscores @@ -631,12 +656,18 @@ def _turn_signal(record: dict[str, Any], harness: str) -> tuple[str, Any] | None # `` is a WRAPPER, not a rejection. All 36 Codex records that open with # one carry real operator text after it, so rejecting on the tag would drop -# genuine prompts. Claude spells the same thing `[Image: source: /path/…]` in -# plain text, and there the opposite holds: 385 of 386 such records are nothing -# but image markers, and they reach the empty-after-stripping rejection instead. -# The 386th spells it `[Image source:` with no colon, which is why the separator -# is a class rather than a literal. The attribute matcher is loose because Codex -# writes `name=[Image #1]` unquoted, spaces and `#` and `]` included. +# genuine prompts. Claude spells the same thing in plain text, in three +# populations that behave differently and were once counted as one: +# +# `[Image: source: /path/…]` 387 records, 387 of them nothing but markers +# `[Image source: /path/…]` 9 records, all 9 nothing but markers +# `[Image #1]` 135 records, 134 carrying operator text after it +# +# The first two reach the empty-after-stripping rejection; the third is why this +# is a wrapper at all, and why the separator is a character class rather than a +# literal colon — `#` follows a space, not a colon. The attribute matcher is +# loose because Codex writes `name=[Image #1]` unquoted, spaces and `#` and `]` +# included. _PROMPT_IMAGE_WRAPPER_RE = re.compile( r"^\s*(?:]*>\s*(?:)?|\[Image[:\s][^\]]*\])\s*", re.IGNORECASE, @@ -664,7 +695,7 @@ def _turn_signal(record: dict[str, Any], harness: str) -> tuple[str, Any] | None # overlooked: a message from another agent is not the operator's instruction, # so the 563 sessions carrying one show nothing from it. -# Measured leading a Codex user-role record; counts are corpus occurrences. +# Measured LEADING a Codex user-role record, over the union described above. _CODEX_USER_TAGS = frozenset( { "recommended_plugins", # 226 @@ -677,7 +708,7 @@ def _turn_signal(record: dict[str, Any], harness: str) -> tuple[str, Any] | None "bash-input", # 14 "bash-stdout", # 14 "user_shell_command", # 5 - "turn_aborted", # 3 + "turn_aborted", # 3 here, and 52 leading a developer-role record } ) @@ -686,29 +717,44 @@ def _turn_signal(record: dict[str, Any], harness: str) -> tuple[str, Any] | None # injection under has already moved once — `turn_aborted` appears under both — # and the cost is asymmetric: an unlisted tag renders harness markup as a # person's words, while a listed one that never arrives costs nothing. +# +# LEADING counts, which is the only kind that can make `injected_prompt` fire. +# These were containment counts before, and the difference is not cosmetic: two +# of the seven lead 0 records and can never fire at all, and reading a +# containment count as evidence for a leading rule is what hid +# `collaboration_mode` — 122 containments, 53 of them leading — until someone +# counted the two separately. The zero pair stays for the asymmetry above, but it +# is labelled rather than left looking measured. _CODEX_DEVELOPER_TAGS = frozenset( { - "permissions", # 410 - "skills_instructions", # 388 - "apps_instructions", # 377 - "plugins_instructions", # 377 - "multi_agent_mode", # 359 + "permissions", # 338 + "multi_agent_mode", # 326 + "skills_instructions", # 80 "collaboration_mode", # 53 - "app-context", # 3 + "app-context", # 2 + "apps_instructions", # 0 leading (281 contained) — defensive only + "plugins_instructions", # 0 leading (244 contained) — defensive only } ) -# Measured leading a Claude user-role record. +# Measured LEADING a Claude user-role record. +# +# Four of these nine — 1,917 of the 5,393 occurrences — sit on records +# `_turn_signal` already refuses, so they earn their place only on the readers +# that do not go through it (`observer._message_from` is the one that matters). +# The overlap is marked per entry rather than pruned: the two predicates answer +# different questions, and a name dropped here because one caller happens to +# reject it would silently un-reject it for the other. _CLAUDE_USER_TAGS = frozenset( { - "task-notification", # 1771 - "teammate-message", # 1176 - "local-command-caveat", # 1064 - "local-command-stdout", # 620 + "task-notification", # 1804 + "teammate-message", # 1184 + "local-command-caveat", # 1065, all refused by `_turn_signal` + "local-command-stdout", # 621, all refused by `_turn_signal` "bash-input", # 242 "bash-stdout", # 241 + "system-reminder", # 227, 223 of them refused by `_turn_signal` "channel", # 8 — a Slack-plugin envelope, request text inside the tag - "system-reminder", # 4 "local-command-stderr", # 1 } ) @@ -725,23 +771,43 @@ def _turn_signal(record: dict[str, Any], harness: str) -> tuple[str, Any] | None # would buy nothing and would make a harness that borrows another's wording # silently wrong. _INJECTED_PROMPT_PREFIXES = ( - "# AGENTS.md instructions", # codex 164 + "# AGENTS.md instructions", # codex 165 "Analyze this conversation and determine", # claude 1084 - "Another Claude session sent a message:", # codex 130, claude 636 - "Base directory for this skill:", # claude 3051 + "Another Claude session sent a message:", # codex 130, claude 645 + "Base directory for this skill:", # claude 3057 "Caveat: The messages below were generated by the user while running", # claude 43 "Stop hook feedback:", # claude 581 - "This session is being continued from a previous conversation", # claude 355 - "[Request interrupted by user", # codex 4, claude 294 + "This session is being continued from a previous conversation", # claude 356 + "[Request interrupted by user", # codex 4, claude 297 "[external_agent_tool_result]", # codex 4 ) -# Matched whole rather than as a prefix. All 97 occurrences are exactly this -# word, and as a prefix it would reject "Warmup the cache before the run", -# which is an operator saying something. +# Matched whole rather than as a prefix. All 97 occurrences over the Claude +# population above are exactly this word, and as a prefix it would reject +# "Warmup the cache before the run", which is an operator saying something. +# +# Subagent-only: all 97 are `isSidechain` records, 0 arrive on a Codex rollout, +# and `_turn_signal` already refuses 12 of the 97. So the rule matters to the +# readers that see a subagent's own transcript and to nothing else. _INJECTED_PROMPTS = frozenset({"Warmup"}) +# Trimmed off both ends before anything is matched against the vocabularies. +# `str.strip()` removes whitespace and NONE of these is whitespace to Python, so +# a single U+FEFF in front of `` defeated the leading-tag regex, +# the prose prefixes and the whole-body set — all three branches of +# `injected_prompt` at once, and it is the one degenerate class that fails OPEN: +# the answer becomes "the operator said this", and harness machinery is published +# as a goal. `safe_text` strips most of the same set, but it runs after this on +# every path that reads a prompt. +# +# The set is `_UNSAFE_CHARS` minus C0/DEL, plus U+2060 and U+FEFF: both are +# invisible joiners no prompt legitimately opens with. U+200C and U+200D stay out +# for the reason `_UNSAFE_CHARS` gives — they are orthographic. +_PROMPT_TRIM_CLASS = "\\s\\ufeff\\u200b\\u2060\\u200e\\u200f\\u202a-\\u202e\\u2066-\\u2069" +_PROMPT_TRIM_RE = re.compile(f"^[{_PROMPT_TRIM_CLASS}]+|[{_PROMPT_TRIM_CLASS}]+$") + + def strip_prompt_wrappers(text: str) -> str: """Peel the harness's image markers off the front of a user prompt. @@ -749,9 +815,9 @@ def strip_prompt_wrappers(text: str) -> str: arrives as its own marker. What is left is either the operator's own words or nothing at all. """ - stripped = text.strip() + stripped = _PROMPT_TRIM_RE.sub("", text) while True: - shorter = _PROMPT_IMAGE_WRAPPER_RE.sub("", stripped, count=1).strip() + shorter = _PROMPT_TRIM_RE.sub("", _PROMPT_IMAGE_WRAPPER_RE.sub("", stripped, count=1)) if shorter == stripped: return stripped stripped = shorter @@ -764,6 +830,11 @@ def injected_prompt(text: str, harness: str) -> bool: compaction summary, an envelope around something else. Callers use it to decide what may stand in for a person's intent, so a false positive costs a real prompt and a false negative reports markup as a goal. + + `observer._is_generic_opener` asks the same question of a different thing — + a real user message that states no goal — and keeps its own short list. The + two are deliberately disjoint and deliberately separate; that function's + docstring owns why. """ body = strip_prompt_wrappers(text) if not body: diff --git a/cargento/skills/cargento/cargento_runtime/spacedock.py b/cargento/skills/cargento/cargento_runtime/spacedock.py index 50a8eb77..fcd517e5 100644 --- a/cargento/skills/cargento/cargento_runtime/spacedock.py +++ b/cargento/skills/cargento/cargento_runtime/spacedock.py @@ -176,6 +176,42 @@ def stage_names(config: RuntimeConfig, lines: list[str]) -> list[str]: return [entry["name"] for entry in stage_entries(config, lines)] +def _codex_tool_output(record: dict[str, Any]) -> list[str] | None: + """A Codex rollout's tool-output text, or None when the record is not one. + + None rather than ``[]`` so the caller can tell "not a Codex record" from "a + Codex tool output that said nothing", and fall through to the Claude and Pi + shapes only in the first case. + + Two payload spellings and two value shapes, all four measured on the local + rollout store rather than inferred from an API description: + ``function_call_output`` carries ``output`` as a string on 15,730 records and + as a list of ``{"type": …, "text": …}`` blocks on 897; + ``custom_tool_call_output`` carries it as a string on 2,956 and as such a + list on 18,477. Of the 23 rollouts naming a ``definition_dir``, the boot + envelope arrives on a ``function_call_output`` in 12. + + Nothing else under ``payload`` is read. A ``function_call``'s own arguments + are the model's request rather than the command's output, which is the + provenance distinction the caller's docstring is about. + """ + payload = record.get("payload") + if record.get("type") != "response_item" or not isinstance(payload, dict): + return None + if payload.get("type") not in ("function_call_output", "custom_tool_call_output"): + return None + output = payload.get("output") + if isinstance(output, str): + return [output] + if not isinstance(output, list): + return [] + return [ + block["text"] + for block in output + if isinstance(block, dict) and isinstance(block.get("text"), str) + ] + + def tool_result_text(record: dict[str, Any]) -> list[str]: """The text of every ``tool_result`` block in one transcript record. @@ -184,15 +220,23 @@ def tool_result_text(record: dict[str, Any]) -> list[str]: conversation text — anything a user pasted or a model echoed — nominate an absolute path for Cargento to open. - Two transcript shapes carry that provenance. Claude writes tool results as + Three transcript shapes carry that provenance. Claude writes tool results as ``content`` blocks with ``type: "tool_result"``. Pi writes them as a - ``toolResult`` role message whose blocks carry ``type: "text"``. - - The two are read exclusively, not additively: a ``toolResult`` role returns - on its own blocks and never falls through to the ``tool_result`` scan below. - Nothing writes both shapes in one message today, so no behaviour changes, - but a transcript that did would lose the second half. + ``toolResult`` role message whose blocks carry ``type: "text"``. Codex writes + neither — it has no ``message`` key at all and puts the echo under + ``payload`` — which is why the observer's stage half was structurally + unreachable there, publishing ``stage: ""`` for every rollout that ran a + workflow. + + The Claude and Pi shapes are read exclusively, not additively: a + ``toolResult`` role returns on its own blocks and never falls through to the + ``tool_result`` scan below. Nothing writes both shapes in one message today, + so no behaviour changes, but a transcript that did would lose the second + half. """ + codex = _codex_tool_output(record) + if codex is not None: + return codex message = record.get("message") if not isinstance(message, dict): return [] diff --git a/cargento/skills/cargento/cargento_runtime/transcripts.py b/cargento/skills/cargento/cargento_runtime/transcripts.py index 798783d8..80dce116 100644 --- a/cargento/skills/cargento/cargento_runtime/transcripts.py +++ b/cargento/skills/cargento/cargento_runtime/transcripts.py @@ -327,6 +327,14 @@ def instruction_from( two lines; Claude's line 1 is a title generated from the opening prompt, so the newest prompt underneath it is the whole point. The flag is the caller's to set because only the caller knows what its own line 1 will hold. + + Deleting the flag and leaning on the frontend's own echo test was measured + and rejected: it emits 213 further "asked" lines across the 458 local + rollouts and `nextInstructionEchoes` suppresses all 213, including the 151 + whose line 1 was clipped at 80 characters — that function's ellipsis clause + is written for exactly that case. Surfacing the withheld characters needs the + frontend rule to change, not this one, and until it does the flag is the + cheaper half of one policy rather than a duplicate of it. """ if prompt is None: return None @@ -363,16 +371,25 @@ def _codex_scan_record(record: dict[str, Any]) -> tuple[str, str, float]: """Classify one Codex rollout record for the instruction scan. BOTH shapes of each thing are read, and that is not belt-and-braces. The - turn-start preamble moved at CLI 0.149: verified across all 457 local + turn-start preamble moved at CLI 0.149: verified across all 458 local rollouts, `event_msg`/`agent_message` with `phase == "commentary"` covers - ~95% of the 306 files on 0.142.5-0.146.1 and **0 of the 88** on 0.149.1, - while `event_msg`/`item_completed` with an `AgentMessage` item covers 86 of - those 88 and none of the older ones. A single-shape reader finds nothing on - the build the operator is actually running. The user record split the same - way — `event_msg`/`user_message` is live in 255 of 457 rollouts and gone by - 0.149.1, `response_item`/message/user is in 456 of 457 — and 0.149 adds a - third, `item_completed` with a `UserMessage` item, which is the only path - carrying the prompt on 4 files. + ~95% of the files on 0.142.5-0.146.1 and none of the 89 on 0.149.x, while + `event_msg`/`item_completed` with an `AgentMessage` item reaches 87 of those + 89 and none of the older ones. A single-shape reader finds nothing on the + build the operator is actually running. + + The narrowing below costs some of that reach and is kept anyway: requiring + `phase == "commentary"` on the item takes it from 87 files to 79, because an + unphased `AgentMessage` is the final answer rather than a statement of + intent, and publishing one under an "agent" label would quote finished work + as current. + + The user record split the same way — `event_msg`/`user_message` is gone by + 0.149.1 and `response_item`/message/user is in all but one rollout — and + 0.149 adds a third, `item_completed` with a `UserMessage` item. That third is + read for the shape rather than for its reach: it appears on 26 files and is + the ONLY path carrying the prompt on **0** of them, so it changes no reading + today and exists so a build that drops the other two still has one. """ payload = records.as_dict(record.get("payload")) at = records.parse_ts(record.get("timestamp") or "") or 0.0 @@ -495,8 +512,8 @@ def codex_instruction(config: RuntimeConfig, state: RuntimeState, path: str) -> local rollouts holding a genuine prompt, 171 (62.0%) have the newest one outside `tail_bytes`, because `reasoning` records carry encrypted blobs that flood the tail. Reverse against forward on the eight largest rollouts - benchmarks 50 ms against 880 ms; the walk measures 2.6 ms median and 93 ms - worst case across all 457 local rollouts. + benchmarks 50 ms against 880 ms; the walk measures 2.5 ms median, 12 ms at + the 95th percentile and 84 ms worst case across all 458 local rollouts. """ try: stat = os.stat(path) diff --git a/cargento/skills/cargento/tests/test_observer.py b/cargento/skills/cargento/tests/test_observer.py index b59c6386..fea6ca2e 100644 --- a/cargento/skills/cargento/tests/test_observer.py +++ b/cargento/skills/cargento/tests/test_observer.py @@ -22,6 +22,7 @@ from typing import Any from unittest import mock +from cargento_runtime import io as runtime_io from cargento_runtime import observer from . import test_page_calm @@ -96,7 +97,7 @@ def _claude_message( def _codex_message( - payload_id: str, + payload_id: str | None, role: str, text: str, *, @@ -107,20 +108,22 @@ def _codex_message( Again the live shape: the id lives at `payload.id` (never at the top level), the text at `payload.content[].text`, and the block type differs by role — `input_text` for the operator, `output_text` for the agent. + + ``payload_id=None`` writes the record with no ``id`` key at all, which is the + MAJORITY shape and not an edge case: 599 of the 784 Codex user-message + records (76.4%) inside the observer's own head and tail windows carry none, + measured over the whole local rollout store. Every fixture here wrote the + id-carrying minority before, which is how the dedup fallback went untested. """ block = "input_text" if role == "user" else "output_text" - return json.dumps( - { - "timestamp": ts, - "type": "response_item", - "payload": { - "id": payload_id, - "type": "message", - "role": role, - "content": [{"type": block, "text": text}], - }, - } - ) + payload: dict[str, Any] = { + "type": "message", + "role": role, + "content": [{"type": block, "text": text}], + } + if payload_id is not None: + payload = {"id": payload_id, **payload} + return json.dumps({"timestamp": ts, "type": "response_item", "payload": payload}) def _codex_session_meta( @@ -344,6 +347,22 @@ def test_ordinary_reporting_prose_is_not_a_block(self) -> None: # shape of those four. "The PR is mergeable and blocked only by required review.", "Your conclusion that live egress is still blocked on the other PR is correct.", + # The last three bare phrases went the same way. Re-measured over + # the whole local Claude corpus the table produced 7 blocks, and + # `not permitted`, `permission denied` and `waiting for your` + # supplied 4 of them — every one inside a quoted or fenced span, two + # of them hitting the 200-character cap with the trigger truncated + # away. Falsifying edit: put any of the three bare phrases back. + 'The hook refused it: the log line was "operation not permitted".', + ( + "The rebase printed `permission denied` for the vendored " + "submodule, which the checkout fixes." + ), + "The release is waiting for your PR to land, so nothing is needed here.", + # `waiting for you` is a prefix of `waiting for your`, so the trailing + # word-boundary test is what makes the line above stop matching. + # Falsifying edit: drop `_BLOCK_TRAILING_RE` from `_indicator_hit`. + "Everyone is waiting for your review of the design doc.", ): with self.subTest(line=line), tempfile.TemporaryDirectory() as tmp: path = self._write_transcript( @@ -392,6 +411,59 @@ def test_a_current_block_is_still_reported(self) -> None: ) self.assertEqual("I am blocked on a missing AWS role.", self.analyze(path)["block"]) + def test_the_first_person_replacements_still_report_a_real_block(self) -> None: + # Narrowing three bare phrases must not cost the case they were there + # for. Each line is the self-state form of one of them, and the last is + # the bare `waiting for you` that survives the word-boundary test the + # possessive now fails. Falsifying edit: delete the first-person entries + # from `_BLOCK_INDICATORS` — every line here loses its block. + for line, expected in ( + ( + "I pulled the manifest. I am not permitted to write to that bucket.", + "I am not permitted to write to that bucket.", + ), + ( + "I read the config. I do not have permission to restart the service.", + "I do not have permission to restart the service.", + ), + ( + "The diff is ready. I am waiting for your decision on the schema.", + "I am waiting for your decision on the schema.", + ), + ("The branch is pushed, waiting for you.", "The branch is pushed, waiting for you."), + ): + with self.subTest(line=line), tempfile.TemporaryDirectory() as tmp: + path = self._write_transcript( + tmp, + [ + _pi_session("fp-002"), + _pi_message("m1", None, "user", "Deploy it"), + _pi_message("m2", "m1", "assistant", line), + ], + ) + self.assertEqual(expected, self.analyze(path)["block"]) + + def test_a_rejected_indicator_hit_does_not_hide_a_later_real_one(self) -> None: + # `str.find` returns the FIRST occurrence, so a possessive earlier in the + # message would otherwise shadow a genuine hand-off after it. Falsifying + # edit: replace `_indicator_hit`'s loop with a single `lower.find`. + with tempfile.TemporaryDirectory() as tmp: + path = self._write_transcript( + tmp, + [ + _pi_session("fp-003"), + _pi_message("m1", None, "user", "Deploy it"), + _pi_message( + "m2", + "m1", + "assistant", + "The release is waiting for your PR to land. " + "The branch is pushed, waiting for you.", + ), + ], + ) + self.assertEqual("The branch is pushed, waiting for you.", self.analyze(path)["block"]) + def test_no_spacedock_withdraws_the_project_reads(self) -> None: # `--no-spacedock` is the switch that turns off the project reads, and # SECURITY.md's project-read contract is written against it. The route @@ -834,25 +906,46 @@ def test_the_newest_directive_is_the_newest_by_stamp_not_by_position(self) -> No ) self.assertEqual("the newest prompt", result["goal"]) + def _two_window_lines(self) -> list[str]: + """A transcript whose goal is only in the head and whose block is only in + the tail, at the shrunk window sizes the caller passes.""" + filler = [ + _claude_message(f"f{i}", "assistant", "x" * 200, ts=f"2026-08-17T02:{i:02d}:00Z") + for i in range(2, 40) + ] + return [ + _claude_message("u1", "user", "the opening prompt", ts="2026-08-17T02:00:00Z"), + *filler, + _claude_message( + "u2", "assistant", "I am blocked on a missing token.", ts="2026-08-17T02:59:00Z" + ), + ] + def test_the_head_window_and_the_tail_window_are_both_read(self) -> None: # The two windows are disjoint on any file over head + tail (465,536 B # with the shipped figures), and the opening directive lives in the head. # The windows are shrunk here rather than the file grown: a 465 KB # fixture proves the same thing and costs a disk write per run. - filler = [ - _claude_message(f"f{i}", "assistant", "x" * 200, ts=f"2026-08-17T02:{i:02d}:00Z") - for i in range(2, 40) - ] - result = self.analyze( - [ - _claude_message("u1", "user", "the opening prompt", ts="2026-08-17T02:00:00Z"), - *filler, - _claude_message("u2", "assistant", "Still going.", ts="2026-08-17T02:59:00Z"), - ], - observer_head_bytes=1_200, - tail_bytes=1_200, - ) + # + # Both readings are asserted, and the second one is the point: this test + # asserted the GOAL alone, which comes out of the head, so stubbing + # `read_tail` to return nothing left it green. The block comes only out + # of the tail, so it fails when either window is not read. + result = self.analyze(self._two_window_lines(), observer_head_bytes=1_200, tail_bytes=1_200) self.assertEqual("the opening prompt", result["goal"]) + self.assertEqual("I am blocked on a missing token.", result["block"]) + + def test_the_tail_assertion_is_load_bearing(self) -> None: + # The falsification, committed rather than described: with `read_tail` + # stubbed out the head still supplies the goal, and only the block + # disappears. If a later edit makes the block reachable from the head, + # this fails and the test above stops proving anything. + with mock.patch.object(runtime_io, "read_tail", return_value=[]): + result = self.analyze( + self._two_window_lines(), observer_head_bytes=1_200, tail_bytes=1_200 + ) + self.assertEqual("the opening prompt", result["goal"]) + self.assertEqual("", result["block"]) def test_a_repeated_prompt_does_not_keep_its_oldest_position(self) -> None: # The dedup key read `record["id"]`, which **0 of 8,312 Claude and 0 of @@ -870,6 +963,63 @@ def test_a_repeated_prompt_does_not_keep_its_oldest_position(self) -> None: ) self.assertEqual("align the release notes", result["goal"]) + def test_a_repeated_codex_prompt_with_no_payload_id_keeps_the_newest(self) -> None: + # The same bug where it actually bites. `payload.id` is absent on 599 of + # the 784 Codex user-message records (76.4%) inside the observer's own + # windows, so the key falls through to the fallback on three records in + # four — and every Codex fixture here used to write the id-carrying + # minority, which is why this went untested. Falsifying edit: make the + # fallback `parsed["text"]` again and the goal becomes "run the + # migration". + result = self.analyze( + [ + _codex_session_meta("019f1c51-6cf9-7981-9a2d-172428800011"), + _codex_message(None, "user", "align the release notes", ts="2026-08-17T02:00:00Z"), + _codex_message(None, "user", "run the migration", ts="2026-08-17T02:01:00Z"), + _codex_message(None, "user", "align the release notes", ts="2026-08-17T02:02:00Z"), + _codex_message(None, "assistant", "On it.", ts="2026-08-17T02:03:00Z"), + ] + ) + self.assertEqual("align the release notes", result["goal"]) + + def test_a_replayed_record_is_still_deduped_by_its_own_id(self) -> None: + # The positional fallback must not cost the case the key exists for: a + # resumed transcript replays earlier records into the new file, carrying + # their original ids and their original stamps. Falsifying edit: drop the + # `seen` set — the replayed opener is then read twice, and the second + # copy is what `directives[-1]` would have to sort past. + lines = [ + _claude_message("u1", "user", "the opening prompt", ts="2026-08-17T02:00:00Z"), + _claude_message("u2", "assistant", "Working.", ts="2026-08-17T02:01:00Z"), + ] + config = dataclasses.replace(self.config) + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "session.jsonl" + path.write_text("\n".join([*lines, lines[0]]) + "\n", encoding="utf-8") + messages = observer._extract_messages(config, str(path)) + self.assertEqual( + [("user", "the opening prompt"), ("assistant", "Working.")], + [(m["role"], m["text"]) for m in messages], + ) + + def test_a_clipped_goal_keeps_its_ellipsis(self) -> None: + # `prompt_title` appends the ellipsis AFTER cutting to the cap, so a + # clipped goal is cap + 1 characters and a scrub at the cap took the `…` + # straight back off — an unmarked mid-token cut on 8 of the 1,295 goals + # the local Claude corpus publishes. Falsifying edit: drop the `+ 1` from + # the `safe_text` bound in `_derive_goal_deterministic`. + # One long token, so `clip` finds no word boundary in the last third and + # cuts hard at the cap: that is the shape whose ellipsis is the 201st + # character and therefore the one the scrub used to remove. + cap = self.config.observer_goal_cap_chars + result = self.analyze( + [ + _claude_message("u1", "user", "x" * (cap + 50)), + _claude_message("u2", "assistant", "On it.", ts="2026-08-17T02:01:00Z"), + ] + ) + self.assertEqual("x" * cap + "…", result["goal"]) + def test_a_tool_result_echo_is_not_a_directive(self) -> None: # A user turn whose content is a tool_result is the harness echoing its # own output back, on every one of the three shapes. @@ -1013,17 +1163,25 @@ def _get(self, server: Any, query: str) -> tuple[int, bytes]: return response.status, body def test_the_route_answers_with_the_sidecar_and_writes_it(self) -> None: + # A CLAUDE-shaped record in the Claude store. It used to be Pi's + # `type: "message"` shape — which occurs 0 times in 3,774 real Claude + # transcripts — so the only end-to-end test of this route exercised a + # parser arm the route can never reach on a real machine. sid = "abcdef12-3456-7890-abcd-ef1234567890" with tempfile.TemporaryDirectory() as tmp: projects = Path(tmp) / "projects" (projects / "-home-me-repo").mkdir(parents=True) (projects / "-home-me-repo" / f"{sid}.jsonl").write_text( - json.dumps( - { - "type": "message", - "id": "m1", - "message": {"role": "user", "content": "Fix the failing build"}, - } + "\n".join( + [ + _claude_message("u1", "user", "Fix the failing build"), + _claude_message( + "u2", + "assistant", + [{"type": "text", "text": "I am blocked on a missing token."}], + ts="2026-08-17T02:01:00Z", + ), + ] ) + "\n", encoding="utf-8", @@ -1055,6 +1213,7 @@ def test_the_route_answers_with_the_sidecar_and_writes_it(self) -> None: self.assertEqual(200, status) payload = json.loads(body) self.assertIn("Fix the failing build", payload["goal"]) + self.assertEqual("I am blocked on a missing token.", payload["block"]) self.assertEqual("", payload["stage"]) # no workflow booted # A sid that resolves to no transcript is a 404, not an empty 200: the # panel must be able to tell "nothing to observe" from "nothing found". @@ -1064,6 +1223,57 @@ def test_the_route_answers_with_the_sidecar_and_writes_it(self) -> None: self.assertEqual(400, bare) self.assertTrue(wrote_sidecar) + def test_the_route_answers_for_a_codex_rollout(self) -> None: + # The route advertises three harnesses and only one of them had an + # end-to-end test. Codex reaches every arm differently: the resolver + # matches `session_meta` rather than a filename stem, the parser reads + # `response_item` rather than `type: "user"`, and the records carry no + # `payload.id` — the majority shape on a real store. + sid = "019f1c51-6cf9-7981-9a2d-172428800012" + with tempfile.TemporaryDirectory() as tmp: + day = Path(tmp) / "sessions" / "2026" / "08" / "17" + day.mkdir(parents=True) + (day / f"rollout-2026-08-17T02-00-00-{sid}.jsonl").write_text( + "\n".join( + [ + _codex_session_meta(sid), + _codex_message(None, "user", "Rewrite the changelog for the release"), + _codex_message( + None, + "assistant", + "I am blocked on a missing token.", + ts="2026-08-17T02:01:00Z", + ), + ] + ) + + "\n", + encoding="utf-8", + ) + home = Path(tmp) / "cargento-home" + with ( + store_patch(CODEX_SESSIONS_DIR=str(Path(tmp) / "sessions")), + mock.patch.dict(os.environ, {"CARGENTO_HOME": str(home)}), + ): + httpd = make_server() + thread = threading.Thread(target=httpd.serve_forever, daemon=True) + thread.start() + try: + status, body = self._get(httpd, f"?harness=codex&sid={sid}") + finally: + httpd.shutdown() + httpd.server_close() + thread.join(timeout=2) + sidecar = observer.sidecar_path(httpd.application.config, "codex", sid) + assert sidecar is not None + wrote_sidecar = os.path.isfile(sidecar) + + self.assertEqual(200, status) + payload = json.loads(body) + self.assertEqual("Rewrite the changelog for the release", payload["goal"]) + self.assertEqual("I am blocked on a missing token.", payload["block"]) + self.assertEqual("", payload["stage"]) # no workflow booted + self.assertTrue(wrote_sidecar) + class ObserverReachabilityTest(PageJsHarness): """That a reader can actually get to the panel, and what happens when they do. diff --git a/cargento/skills/cargento/tests/test_records.py b/cargento/skills/cargento/tests/test_records.py index 8d0d073b..199dde63 100644 --- a/cargento/skills/cargento/tests/test_records.py +++ b/cargento/skills/cargento/tests/test_records.py @@ -433,6 +433,38 @@ def test_warmup_is_matched_whole_not_as_a_prefix(self) -> None: self.assertTrue(records.injected_prompt("Warmup", "claude")) self.assertFalse(records.injected_prompt("Warmup the cache before the run", "claude")) + def test_a_leading_invisible_character_does_not_defeat_every_branch(self) -> None: + """The one degenerate class that failed OPEN. + + ``str.strip()`` removes whitespace and none of these is whitespace to + Python, so a single one in front of a record defeated the leading-tag + regex, the prose prefixes and the whole-body set at once — all three + branches — and `injected_prompt` answered "the operator said this" about + the harness's own machinery. `safe_text` strips most of the same set, but + it runs after this on every path that reads a prompt. + + Falsifying edit: drop `_PROMPT_TRIM_RE` from `strip_prompt_wrappers`. + """ + for name, lead in ( + ("BOM", "\ufeff"), + ("zero-width space", "\u200b"), + ("word joiner", "\u2060"), + ("LRM", "\u200e"), + ("RLO", "\u202e"), + ("two of them", "\ufeff\u2060"), + ("one either side of a space", "\ufeff \u2060"), + ): + for body in ( + "x", + "Stop hook feedback: retry", + "Warmup", + ): + with self.subTest(lead=name, body=body): + self.assertTrue(records.injected_prompt(lead + body, "claude")) + # And the other direction: the trim must not turn a real prompt into an + # injection, which it would if it ate anything visible. + self.assertFalse(records.injected_prompt("\ufeffFix the flaky Windows test", "claude")) + def test_operator_text_survives(self) -> None: for text in ( "Fix the flaky Windows test", diff --git a/cargento/skills/cargento/tests/test_spacedock.py b/cargento/skills/cargento/tests/test_spacedock.py index b8616f27..8b8dba5b 100644 --- a/cargento/skills/cargento/tests/test_spacedock.py +++ b/cargento/skills/cargento/tests/test_spacedock.py @@ -235,6 +235,99 @@ def test_boot_records_finds_pi_tool_result_format(self) -> None: self.assertEqual(1, len(records)) self.assertEqual("/w/one", records[0]["definition_dir"]) + def test_boot_records_finds_codex_tool_output_format(self) -> None: + """Codex writes tool output under ``payload``, in two spellings and two + value shapes, and had no branch at all — so the observer's stage half was + structurally unreachable there and every rollout running a workflow + published ``stage: ""``. + + All four shapes are measured on the local rollout store rather than read + off an API description: ``function_call_output`` carries ``output`` as a + string on 15,730 records and as a block list on 897, + ``custom_tool_call_output`` as a string on 2,956 and as a block list on + 18,477. Falsifying edit: remove the ``_codex_tool_output`` call from + ``tool_result_text`` — every arm below returns []. + """ + config, _runtime = runtime() + envelope = ( + '{"command":"boot","id_style":"slug",' + '"definition_dir":"/w/one","entity_dir":"/w/one",' + '"dispatchable":[{"slug":"drc-1","current":"review"}]}' + ) + text = "=== BOOT ===\n" + envelope + + def line(payload_type: str, output: Any) -> bytes: + return json.dumps( + { + "timestamp": "2026-08-21T00:00:00Z", + "type": "response_item", + "payload": {"type": payload_type, "call_id": "call-1", "output": output}, + } + ).encode() + + for payload_type in ("function_call_output", "custom_tool_call_output"): + for label, output in ( + ("string", text), + ("blocks", [{"type": "input_text", "text": text}]), + ): + with self.subTest(payload=payload_type, shape=label): + found = spacedock.boot_records(config, line(payload_type, output)) + self.assertEqual(1, len(found)) + self.assertEqual("/w/one", found[0]["definition_dir"]) + self.assertEqual({"drc-1": "review"}, spacedock.boot_entities(found, "/w/one")) + + def test_codex_conversation_text_cannot_nominate_a_path(self) -> None: + """The negative twin: the provenance rule is the same one Pi's branch + carries, and a Codex record that is not a tool OUTPUT must not nominate a + directory. A ``function_call``'s arguments are the model's request, and a + message is a person or a model talking. Falsifying edit: drop the payload + type gate in ``_codex_tool_output``. + """ + config, _runtime = runtime() + envelope = ( + '{"command":"boot","id_style":"slug",' + '"definition_dir":"/w/one","entity_dir":"/w/one",' + '"dispatchable":[]}' + ) + for payload_type in ("function_call", "custom_tool_call", "message", "reasoning"): + with self.subTest(payload=payload_type): + record = json.dumps( + { + "type": "response_item", + "payload": { + "type": payload_type, + "output": envelope, + "arguments": envelope, + "content": [{"type": "input_text", "text": envelope}], + }, + } + ).encode() + self.assertEqual([], spacedock.boot_records(config, record)) + + def test_a_codex_tool_output_block_that_is_not_a_string_is_skipped(self) -> None: + """The same isinstance guard the Pi branch needs, on the Codex arm. + + A block whose ``text`` is an object reaches ``str.find`` in the boot + scanner otherwise, and the ``AttributeError`` escapes the collector to + blank every row for that harness. An ``output`` that is neither a string + nor a list is the same class of untrusted value. Falsifying edit: drop + the isinstance checks from ``_codex_tool_output``. + """ + config, _runtime = runtime() + for output in ([{"type": "input_text", "text": {"definition_dir": "/w/x"}}], 7, None): + with self.subTest(output=type(output).__name__): + record = json.dumps( + { + "type": "response_item", + "payload": { + "type": "function_call_output", + "output": output, + "note": "definition_dir", + }, + } + ).encode() + self.assertEqual([], spacedock.boot_records(config, record)) + def test_pi_conversation_text_cannot_nominate_a_path(self) -> None: """The negative twin of the Pi format test above. diff --git a/scripts/derive_prompt_shapes.py b/scripts/derive_prompt_shapes.py new file mode 100755 index 00000000..e0342563 --- /dev/null +++ b/scripts/derive_prompt_shapes.py @@ -0,0 +1,411 @@ +#!/usr/bin/env python3 +"""Re-derive the prompt-shape counts written into `records.py`. + +Every number in the "Harness-injected prompts" comment block of +`cargento_runtime/records.py` was measured against a real local store. Until this +script existed the measuring code did not survive the measurement, so no reviewer +could tell a derived count from a plausible one. This is that code, kept. + + python3 scripts/derive_prompt_shapes.py # default roots + python3 scripts/derive_prompt_shapes.py --claude-root DIR --codex-root DIR + python3 scripts/derive_prompt_shapes.py --no-codex # one half only + +**It prints counts and shape names, never prompt text.** That is a deliverable +constraint, not a courtesy: a transcript store holds whatever the operator typed, +including credentials, and a derivation script is the one tool whose whole job is +to read all of it. So every emitted label goes through `_safe_label`, which +accepts a markup tag name (`[A-Za-z][A-Za-z0-9_-]*`), a literal already present in +`records.py`'s own vocabularies, or a fixed structural label defined below — +and raises on anything else. Discovering a *new* prose prefix would require +printing prose, so it is deliberately out of scope; the tag half discovers new +names on its own because a tag name is markup rather than text. + +`records._HARNESS_CONTROL_COMMANDS` is also out of scope. Its counts are +"occurrences as the last published GOAL", which is a derivation over the +observer's session-level pick rather than a shape scan over records, so it needs +a different instrument than this one. +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from collections import Counter +from pathlib import Path +from typing import Any + +ROOT = Path(__file__).resolve().parents[1] +SKILL_DIR = ROOT / "cargento" / "skills" / "cargento" +if str(SKILL_DIR) not in sys.path: + sys.path.insert(0, str(SKILL_DIR)) + +from cargento_runtime import records # noqa: E402 + +DEFAULT_CLAUDE_ROOT = Path.home() / ".claude" / "projects" +DEFAULT_CODEX_ROOT = Path.home() / ".codex" / "sessions" + +# The collectors' own globs, so the population this reports is the population the +# dashboard actually reads rather than a wider sweep of the same directory. +CLAUDE_GLOB = "*/*.jsonl" +CODEX_GLOB = "*/*/*/rollout-*.jsonl" + +# A markup tag name, and short enough to be one. The length bound is the part +# that matters: without it a hyphenated run with no spaces in it — which is what +# a prompt looks like once it is one long token — reads as a tag name and would +# be printed. The longest name in any of `records.py`'s vocabularies is 21 +# characters. +_TAG_NAME_RE = re.compile(r"^[A-Za-z][A-Za-z0-9_-]{0,39}$") + +# Leading image-marker populations, reported separately because they are three +# populations and were once counted as one. `[Image #N]` is Codex's spelling +# reaching a Claude record through a pasted screenshot; the other two are +# Claude's own. +_IMAGE_SHAPES = ( + ("[Image: …]", re.compile(r"^\s*\[Image:", re.IGNORECASE)), + ("[Image source…]", re.compile(r"^\s*\[Image source", re.IGNORECASE)), + ("[Image #N]", re.compile(r"^\s*\[Image\s*#", re.IGNORECASE)), + (" (Codex tag)", re.compile(r"^\s* str: + """One emitted label, or a raise. + + The guard the module docstring promises. Anything that is not a markup tag + name, a literal `records.py` already carries, or a structural label from the + fixed set above could be operator text, and this script never prints that. + """ + if ( + _TAG_NAME_RE.match(label) + or label in _STRUCTURAL_LABELS + or label in records._INJECTED_PROMPTS # noqa: SLF001 - the vocabulary is the subject + or label in records._INJECTED_PROMPT_PREFIXES # noqa: SLF001 - same + ): + return label + raise UnsafeLabelError(label) + + +def _emit(label: str, count: int, *, indent: int = 2) -> None: + print(f"{' ' * indent}{_safe_label(label):<52} {count}") + + +def _records(path: Path) -> list[dict[str, Any]]: + """Every JSON object in one JSONL file, malformed lines dropped.""" + out: list[dict[str, Any]] = [] + try: + raw = path.read_bytes() + except OSError: + return out + for line in raw.split(b"\n"): + if not line.startswith(b"{"): + continue + try: + record = json.loads(line) + except ValueError: + continue + if isinstance(record, dict): + out.append(record) + return out + + +def _leading_tag(text: str) -> str | None: + """The tag name `injected_prompt` would read off this text, or None. + + The same two steps that function takes, in the same order: image wrappers off + the front first, then the leading-tag match on what is left. + """ + body = records.strip_prompt_wrappers(text) + if not body: + return None + match = records._PROMPT_LEADING_TAG_RE.match(body) # noqa: SLF001 - the regex is the subject + if match is None: + return None + name = match.group(1).casefold() + return name if _TAG_NAME_RE.match(name) else _OVER_LONG_TAG + + +class TagTally: + """Leading, containment and refusal counts for one harness's tag vocabulary. + + Three counters and not one, because the shipped comment carried a single + number that was silently a containment count: a tag that never *leads* can + never make `injected_prompt` fire, so a containment count reads as evidence + for a rule it cannot support. + """ + + def __init__(self) -> None: + self.leading: Counter[str] = Counter() + self.contained: Counter[str] = Counter() + self.refused: Counter[str] = Counter() + + def add(self, text: str, vocabulary: frozenset[str], *, refused: bool) -> None: + tag = _leading_tag(text) + if tag is not None: + self.leading[tag] += 1 + if refused: + self.refused[tag] += 1 + lowered = text.casefold() + for name in vocabulary: + if f"<{name}" in lowered: + self.contained[name] += 1 + + def report(self, vocabulary: frozenset[str], *, title: str) -> None: + print(f"\n{title}") + for name in sorted(vocabulary, key=lambda n: (-self.leading[n], n)): + leading = self.leading[name] + contained = self.contained[name] + refused = self.refused[name] + suffix = f" (contained {contained}, refused {refused})" + print(f" {_safe_label(name):<52} {leading}{suffix}") + unlisted = [ + (name, count) + for name, count in self.leading.most_common() + if name not in vocabulary and count > 1 + ] + if unlisted: + print(" -- leading tags NOT in the vocabulary (count > 1) --") + for name, count in unlisted: + _emit(name, count) + + +def _claude_refuses(record: dict[str, Any], content: Any) -> bool: + """Whether `records._turn_signal` would drop this Claude user record. + + Reimplemented rather than called, because `_turn_signal` returns a signal and + this needs the reason. Kept in step with it by + `scripts/tests/test_derive_prompt_shapes.py`, which asserts the two agree. + """ + if record.get("isMeta"): + return True + if isinstance(content, list) and any( + isinstance(item, dict) and item.get("type") == "tool_result" for item in content + ): + return True + return isinstance(content, str) and content.lstrip().startswith( + ("", "") + ) + + +class ProseTally: + """The prefix, whole-body and image-marker counts, for one harness. + + Shared by both halves because the three vocabularies are shared: splitting + them per harness is exactly the thing the comment block in `records.py` says + was considered and rejected, so the instrument must count them the same way + on both sides or the comparison is meaningless. + + ``flagged`` is whatever narrower population that half wants counted beside + the whole-body hits — sidechain records on Claude, subagent rollouts on + Codex — because "97, and all 97 are subagent-only" is the claim, not "97". + """ + + def __init__(self) -> None: + self.prefixes: Counter[str] = Counter() + self.whole: Counter[str] = Counter() + self.images: Counter[str] = Counter() + self.empty: Counter[str] = Counter() + self.flagged = 0 + + def add(self, text: str, *, flagged: bool) -> None: + body = records.strip_prompt_wrappers(text) + for prefix in records._INJECTED_PROMPT_PREFIXES: # noqa: SLF001 - the vocabulary is the subject + if body.startswith(prefix): + self.prefixes[prefix] += 1 + if body in records._INJECTED_PROMPTS: # noqa: SLF001 - same + self.whole[body] += 1 + self.flagged += int(flagged) + for name, pattern in _IMAGE_SHAPES: + if pattern.match(text): + self.images[name] += 1 + self.empty[name] += int(not body) + + def report(self, half: str, flag_label: str) -> None: + print("\nleading image markers") + for name, _ in _IMAGE_SHAPES: + carries = self.images[name] - self.empty[name] + print( + f" {_safe_label(name):<52} {self.images[name]}" + f" ({_safe_label('strips to empty')} {self.empty[name]}," + f" {_safe_label('carries operator text')} {carries})" + ) + print(f"\ninjected prose prefixes ({half})") + for prefix in records._INJECTED_PROMPT_PREFIXES: # noqa: SLF001 - the vocabulary is the subject + _emit(prefix, self.prefixes[prefix]) + print(f"\nwhole-body injections ({half})") + for value in sorted(records._INJECTED_PROMPTS): # noqa: SLF001 - same + _emit(value, self.whole[value]) + _emit(flag_label, self.flagged, indent=4) + + +def derive_claude(root: Path) -> None: + """Counts for `_CLAUDE_USER_TAGS` and the Claude half of the prose lists.""" + files = sorted(root.glob(CLAUDE_GLOB)) + tags = TagTally() + prose = ProseTally() + texts = 0 + for path in files: + for record in _records(path): + if record.get("type") != "user": + continue + content = records.message_dict(record).get("content") + text = records.extract_text(content) + if not text: + continue + texts += 1 + tags.add(text, records._CLAUDE_USER_TAGS, refused=_claude_refuses(record, content)) # noqa: SLF001 + prose.add(text, flagged=bool(record.get("isSidechain"))) + + print("\n=== Claude ===") + _emit("files scanned", len(files)) + _emit("user-role texts", texts) + tags.report(records._CLAUDE_USER_TAGS, title="_CLAUDE_USER_TAGS (leading occurrences)") # noqa: SLF001 + prose.report("Claude half", "in sidechain records") + + +def _codex_texts(record: dict[str, Any]) -> tuple[str, str]: + """``(population, text)`` for one Codex rollout record, or ``("", "")``. + + Three populations, because the vocabularies were derived from two of them and + the third is what makes the occurrence counts double-count: a Codex build + writes the operator's prompt as an `event_msg`/`user_message` AND again as a + `response_item` message, so one prompt is two records. + """ + payload = records.as_dict(record.get("payload")) + outer = record.get("type") + if outer == "event_msg" and payload.get("type") == "user_message": + return ("event_msg user_message texts", records.extract_text(payload.get("message"))) + if outer == "response_item" and payload.get("type") == "message": + role = payload.get("role") + if role == "user": + return ("response_item user texts", records.extract_text(payload.get("content"))) + if role == "developer": + return ("developer-role texts", records.extract_text(payload.get("content"))) + return ("", "") + + +DEVELOPER_POPULATION = "developer-role texts" +EVENT_MSG_POPULATION = "event_msg user_message texts" +RESPONSE_POPULATION = "response_item user texts" + + +def _dual_written(event_msg_texts: list[str], response_texts: set[str]) -> int: + """How many of one rollout's `event_msg` prompts are also `response_item`s. + + Matched on the first 400 characters, not on equality: `extract_text` joins a + `response_item`'s content blocks and caps the join at 2,000 characters while + an `event_msg`'s `message` is one uncapped string, so a long prompt's two + spellings differ past that cap. Equality scores 850 of 1,007 across the local + store where the head match scores 974. + """ + heads = {text[:400] for text in response_texts} + return sum(1 for text in event_msg_texts if text[:400] in heads) + + +def derive_codex(root: Path) -> None: + """Counts for the two Codex tag sets and the Codex half of the prose list.""" + files = sorted(root.glob(CODEX_GLOB)) + tallies = { + DEVELOPER_POPULATION: (TagTally(), records._CODEX_DEVELOPER_TAGS), # noqa: SLF001 + EVENT_MSG_POPULATION: (TagTally(), records._CODEX_USER_TAGS), # noqa: SLF001 + } + # The two user populations share one tally: the vocabulary was derived from + # their union, which is what makes its counts occurrences rather than prompts. + tallies[RESPONSE_POPULATION] = tallies[EVENT_MSG_POPULATION] + prose = ProseTally() + counts: Counter[str] = Counter() + dual = 0 + for path in files: + event_msg_texts: list[str] = [] + response_texts: set[str] = set() + subagent = False + for record in _records(path): + if record.get("type") == "session_meta": + subagent = records.as_dict(record.get("payload")).get("thread_source") == "subagent" + population, text = _codex_texts(record) + if not population or not text: + continue + counts[population] += 1 + if population == EVENT_MSG_POPULATION: + event_msg_texts.append(text) + elif population == RESPONSE_POPULATION: + response_texts.add(text) + tally, vocabulary = tallies[population] + tally.add(text, vocabulary, refused=False) + prose.add(text, flagged=subagent) + dual += _dual_written(event_msg_texts, response_texts) + + print("\n=== Codex ===") + _emit("files scanned", len(files)) + for population in (EVENT_MSG_POPULATION, RESPONSE_POPULATION, DEVELOPER_POPULATION): + _emit(population, counts[population]) + _emit("dual-written event_msg prompts", dual) + tallies[EVENT_MSG_POPULATION][0].report( + records._CODEX_USER_TAGS, # noqa: SLF001 + title="_CODEX_USER_TAGS (leading occurrences)", + ) + tallies[DEVELOPER_POPULATION][0].report( + records._CODEX_DEVELOPER_TAGS, # noqa: SLF001 + title="_CODEX_DEVELOPER_TAGS (leading occurrences)", + ) + prose.report("Codex half", "in subagent rollouts") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__ or "") + parser.add_argument("--claude-root", type=Path, default=DEFAULT_CLAUDE_ROOT) + parser.add_argument("--codex-root", type=Path, default=DEFAULT_CODEX_ROOT) + parser.add_argument("--no-claude", action="store_true") + parser.add_argument("--no-codex", action="store_true") + args = parser.parse_args(argv) + print("counts and shape names only — this script never prints prompt text") + if not args.no_claude: + if args.claude_root.is_dir(): + derive_claude(args.claude_root) + else: + print(f"\n=== Claude ===\n no store at {args.claude_root}") + if not args.no_codex: + if args.codex_root.is_dir(): + derive_codex(args.codex_root) + else: + print(f"\n=== Codex ===\n no store at {args.codex_root}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/test_derive_prompt_shapes.py b/scripts/tests/test_derive_prompt_shapes.py new file mode 100644 index 00000000..658fcda7 --- /dev/null +++ b/scripts/tests/test_derive_prompt_shapes.py @@ -0,0 +1,211 @@ +"""The prompt-shape derivation script. + +Two things are worth testing here and they are not the counts. The counts are the +script's output over whoever's store it is pointed at, and a fixture store can +only prove the arithmetic reaches the right column. What matters is: + + 1. **It never prints prompt text.** That is a deliverable constraint, and a + guard nobody tests is a guard that stops holding the first time a label is + added. The fixtures below seed distinctive strings and the tests assert the + whole stdout carries none of them. + 2. **Its `_turn_signal` reimplementation agrees with the real one.** The + refusal column in `records.py`'s comments is derived from that copy, so a + copy that drifts publishes a wrong provenance claim. +""" + +from __future__ import annotations + +import io +import json +import sys +import tempfile +import unittest +from contextlib import contextmanager, redirect_stdout +from pathlib import Path +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from collections.abc import Iterator + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "cargento" / "skills" / "cargento")) + +import derive_prompt_shapes as derive +from cargento_runtime import records + +# Nothing a real transcript would contain, so a leak is unambiguous. Named for +# what it is rather than for what it stands in for: a variable called +# SECRET_ anything trips ruff's hardcoded-credential rule. +SEEDED_PROMPT = "zzqq operator typed this into the prompt box" + + +def _claude_record(content: Any, **extra: Any) -> str: + record: dict[str, Any] = { + "type": "user", + "uuid": "u1", + "message": {"role": "user", "content": content}, + } + record.update(extra) + return json.dumps(record) + + +def _codex_record(role: str, text: str) -> str: + return json.dumps( + { + "type": "response_item", + "payload": { + "type": "message", + "role": role, + "content": [{"type": "input_text", "text": text}], + }, + } + ) + + +def _seed(root: Path) -> tuple[Path, Path]: + """A two-harness fixture store shaped like the globs the script uses.""" + claude = root / "claude" / "projects" / "-home-me-repo" + claude.mkdir(parents=True) + (claude / "session.jsonl").write_text( + "\n".join( + [ + _claude_record(SEEDED_PROMPT), + _claude_record("done"), + _claude_record("x"), + _claude_record("[Image: source: /tmp/a.png]"), + _claude_record("Warmup", isSidechain=True), + _claude_record("Stop hook feedback: " + SEEDED_PROMPT), + ] + ) + + "\n", + encoding="utf-8", + ) + codex = root / "codex" / "sessions" / "2026" / "08" / "17" + codex.mkdir(parents=True) + (codex / "rollout-2026-08-17T02-00-00-abc.jsonl").write_text( + "\n".join( + [ + _codex_record("user", SEEDED_PROMPT), + _codex_record("user", "list"), + _codex_record("developer", "x"), + _codex_record("developer", "x"), + ] + ) + + "\n", + encoding="utf-8", + ) + return (root / "claude" / "projects", root / "codex" / "sessions") + + +@contextmanager +def _temp_root() -> Iterator[Path]: + with tempfile.TemporaryDirectory() as tmp: + yield Path(tmp) + + +class OutputTest(unittest.TestCase): + def _run(self, root: Path) -> str: + claude_root, codex_root = _seed(root) + buffer = io.StringIO() + with redirect_stdout(buffer): + code = derive.main(["--claude-root", str(claude_root), "--codex-root", str(codex_root)]) + self.assertEqual(0, code) + return buffer.getvalue() + + def test_no_prompt_text_reaches_stdout(self) -> None: + # The deliverable constraint. A derivation script reads every prompt the + # operator ever typed, credentials included, so the one thing it may + # never do is echo one. + with _temp_root() as root: + output = self._run(root) + self.assertNotIn(SEEDED_PROMPT, output) + self.assertIn("never prints prompt text", output) + + def test_the_counts_land_in_the_right_columns(self) -> None: + with _temp_root() as root: + output = self._run(root) + # Shape names and totals only; each of these is one seeded record. + self.assertIn("user-role texts 6", output) + self.assertIn("files scanned 1", output) + # A leading tag is counted as leading; a developer tag that only ever + # appears contained is counted as contained. + self.assertRegex(output, r"task-notification\s+1\s+\(contained 1, refused 0\)") + self.assertRegex(output, r"local-command-caveat\s+1\s+\(contained 1, refused 1\)") + self.assertRegex(output, r"apps_instructions\s+1\s+\(contained 1") + self.assertRegex(output, r"Warmup\s+1") + + def test_a_missing_store_is_reported_rather_than_crashed(self) -> None: + buffer = io.StringIO() + with redirect_stdout(buffer): + code = derive.main( + ["--claude-root", "/nonexistent/a", "--codex-root", "/nonexistent/b"] + ) + self.assertEqual(0, code) + self.assertIn("no store at", buffer.getvalue()) + + +class SafeLabelTest(unittest.TestCase): + def test_a_label_that_could_be_prompt_text_raises(self) -> None: + # The guard is a whitelist, so anything new has to be added to it + # deliberately rather than printed because it happened to be short. + for label in ( + SEEDED_PROMPT, + "fix the build", + "", + "