From d53a52f484296ebbd878d671cd6b872ec9ac05ae Mon Sep 17 00:00:00 2001 From: Jared Scott Date: Wed, 16 Sep 2026 11:05:20 +0800 Subject: [PATCH] fix(api,web): resolve dismissal cleared field, ended header tense, and unobservable ends - dismissals: reject Claude session IDs longer than 8 characters to prevent unbounded unmatchable keys from evicting legitimate dismissals. - http_api: remove the obsolete cleared field from the POST /api/dismiss response payload. - next-session.js: yield present-tense state phrases and active duration to observed session ends in session meta header. - aggregate and next-observed.js: report ends unobservable when the session-ends store is not observable. - tests: update byte oracle pins and add unit coverage across dismissals, http_api, next_session, next_observed, and next_attention. Fixes DRC-4179, DRC-4554, DRC-4555. Signed-off-by: Jared Scott --- .../cargento/cargento_runtime/aggregate.py | 1 + .../cargento/cargento_runtime/dismissals.py | 20 +++++++++-- .../cargento/cargento_runtime/http_api.py | 1 - .../cargento_runtime/web/next-observed.js | 2 +- .../cargento_runtime/web/next-session.js | 36 ++++++++++--------- .../skills/cargento/tests/test_dismissals.py | 34 ++++++++++++++++++ cargento/skills/cargento/tests/test_focus.py | 2 +- .../skills/cargento/tests/test_http_api.py | 24 +++++++++++-- .../cargento/tests/test_next_attention.py | 12 +++++-- .../skills/cargento/tests/test_next_flag.py | 4 +-- .../cargento/tests/test_next_observed.py | 14 ++++++++ .../skills/cargento/tests/test_next_page.py | 12 +++---- .../cargento/tests/test_next_session.py | 15 ++++++++ 13 files changed, 143 insertions(+), 34 deletions(-) diff --git a/cargento/skills/cargento/cargento_runtime/aggregate.py b/cargento/skills/cargento/cargento_runtime/aggregate.py index 6c3e3034..dfcb4fa1 100644 --- a/cargento/skills/cargento/cargento_runtime/aggregate.py +++ b/cargento/skills/cargento/cargento_runtime/aggregate.py @@ -1253,6 +1253,7 @@ def _attach_command_reports(self, out_sessions: list[Session]) -> dict[str, Any] if (report["harness"], report["sid"]) == (session["harness"], session["sid"]) ] return { + "ends_observable": self.overlays is not None, "irreversible_enabled": enabled, **({"command_reports": reports} if enabled else {}), } diff --git a/cargento/skills/cargento/cargento_runtime/dismissals.py b/cargento/skills/cargento/cargento_runtime/dismissals.py index d67d701e..876a0d43 100644 --- a/cargento/skills/cargento/cargento_runtime/dismissals.py +++ b/cargento/skills/cargento/cargento_runtime/dismissals.py @@ -66,6 +66,20 @@ def store_path(config: RuntimeConfig) -> str: return os.path.join(config.state_home, "cargento-dismissals.json") +def is_matchable(harness: str, sid: str) -> bool: + """Whether (harness, sid) can ever match a published session row. + + Claude Code sessions publish the 8-character transcript prefix as their + sid, so a 36-character UUID or other non-8-character identifier can never + match any published Claude row. Admitting an unmatchable key would write a + permanent junk entry that never expires on activity, consuming one of the + bounded dismissal slots and eventually evicting a real dismissal (DRC-4179). + """ + if not harness or not sid: + return False + return not (harness == "claude" and len(sid) > 8) + + def _entry(value: Any) -> Dismissal | None: """One untrusted record as a dismissal, or nothing. @@ -79,7 +93,7 @@ def _entry(value: Any) -> Dismissal | None: return None harness = records.safe_text(value.get("harness"), KEY_CAP_CHARS).strip() sid = records.safe_text(value.get("sid"), KEY_CAP_CHARS).strip() - if not harness or not sid: + if not is_matchable(harness, sid): return None return { "harness": harness, @@ -258,7 +272,7 @@ def dismiss( if not config.dismissals_enabled: return False key = _key(harness, sid) - if not key[0] or not key[1]: + if not is_matchable(key[0], key[1]): return False stamp = time.time() if now is None else now entry: Dismissal = { @@ -290,6 +304,8 @@ def restore( if not config.dismissals_enabled: return False key = _key(harness, sid) + if not is_matchable(key[0], key[1]): + return False with state.dismissal_lock: bounded = tuple(e for e in load(config) if (e["harness"], e["sid"]) != key) state.dismissals = _stored(bounded) diff --git a/cargento/skills/cargento/cargento_runtime/http_api.py b/cargento/skills/cargento/cargento_runtime/http_api.py index d4d6d8f1..15869c9d 100644 --- a/cargento/skills/cargento/cargento_runtime/http_api.py +++ b/cargento/skills/cargento/cargento_runtime/http_api.py @@ -1201,7 +1201,6 @@ def _dismiss(self) -> None: answer = { "ok": True, "persisted": persisted, - "cleared": len(dismissals.active(config, state)), } self._send(json.dumps(answer, separators=(",", ":")).encode(), "application/json") diff --git a/cargento/skills/cargento/cargento_runtime/web/next-observed.js b/cargento/skills/cargento/cargento_runtime/web/next-observed.js index 790b273d..20e2c853 100644 --- a/cargento/skills/cargento/cargento_runtime/web/next-observed.js +++ b/cargento/skills/cargento/cargento_runtime/web/next-observed.js @@ -472,7 +472,7 @@ function nextObserved(payload, evidence){ quiet: `The other ${other.length}: ${otherWords.filter(word => word[1]).map(word => `${word[1]} ${word[0]}`).join(" · ") || "none"}; ` + `of these, ${partial} partially read.`, gates: `${totals.reportsBlock} of ${totals.sessions} ${totals.sessions === 1 ? "session reports" : "sessions report"} block state · ` + - `${totals.sessions - totals.reportsBlock} unknown · ends observed on ${totals.ended} ${totals.ended === 1 ? "session" : "sessions"}`, + `${totals.sessions - totals.reportsBlock} unknown · ${payload.ends_observable === false ? "ends unobservable" : `ends observed on ${totals.ended} ${totals.ended === 1 ? "session" : "sessions"}`}`, rows: harnesses.map(row => ({key: String(row.key || ""), label: nextObservedString(row.label) || String(row.key || "Harness not published"), sessions: sessions.filter(session => session.harness === row.key).length, ...nextObservedPair("block", !row.error && row.reports_needs_input === true ? diff --git a/cargento/skills/cargento/cargento_runtime/web/next-session.js b/cargento/skills/cargento/cargento_runtime/web/next-session.js index 056a4954..6effc6a1 100644 --- a/cargento/skills/cargento/cargento_runtime/web/next-session.js +++ b/cargento/skills/cargento/cargento_runtime/web/next-session.js @@ -112,24 +112,26 @@ function nextSessionMeta(session){ const parts = []; const harness = nextSessionRegistryLabel(session); if(harness) parts.push(harness); - if(session.state_detail) parts.push(String(session.state_detail)); - /* Outside the state chain below, on purpose. An end is a fact about the - session id; `state` is a reading of file recency that a session which ended - seconds ago can still make say "working". */ + /* An observed end supersedes the present-tense activity and duration phrases; + an ended session must not describe itself as awaiting input (DRC-4554). */ const ended = nextDurationSince(nextSessionEndedAt(session)); - if(ended != null) parts.push(`ended ${ended} ago`); - if(session.state === "needs_input"){ - const blocked = nextDurationSince(session.blocked_since); - if(blocked != null) parts.push(`blocked ${blocked}`); - if(session.wait_unconfirmed) parts.push("unconfirmed: no positive observation in 5m"); - }else if(session.state === "working"){ - const turn = session.turn; - const elapsed = turn && typeof turn === "object" && !Array.isArray(turn) && - typeof turn.elapsed_h === "string" ? turn.elapsed_h.trim() : ""; - if(elapsed) parts.push(`turn started ${elapsed} ago`); - }else if(session.state === "idle"){ - const started = nextDurationSince(session.started_at); - if(started != null) parts.push(`session started ${started} ago`); + if(ended != null){ + parts.push(`ended ${ended} ago`); + }else{ + if(session.state_detail) parts.push(String(session.state_detail)); + if(session.state === "needs_input"){ + const blocked = nextDurationSince(session.blocked_since); + if(blocked != null) parts.push(`blocked ${blocked}`); + if(session.wait_unconfirmed) parts.push("unconfirmed: no positive observation in 5m"); + }else if(session.state === "working"){ + const turn = session.turn; + const elapsed = turn && typeof turn === "object" && !Array.isArray(turn) && + typeof turn.elapsed_h === "string" ? turn.elapsed_h.trim() : ""; + if(elapsed) parts.push(`turn started ${elapsed} ago`); + }else if(session.state === "idle"){ + const started = nextDurationSince(session.started_at); + if(started != null) parts.push(`session started ${started} ago`); + } } /* These last two are unconditional, and both for the reason the first one gives: every clause above is a reading, and these say what the readings diff --git a/cargento/skills/cargento/tests/test_dismissals.py b/cargento/skills/cargento/tests/test_dismissals.py index 890704b5..2f64da0c 100644 --- a/cargento/skills/cargento/tests/test_dismissals.py +++ b/cargento/skills/cargento/tests/test_dismissals.py @@ -362,5 +362,39 @@ def test_the_defaults_are_bounded(self) -> None: self.assertIs(True, dataclasses.replace(config, dismissals_enabled=True).dismissals_enabled) +class MatchableKeyTest(DismissalStoreTestCase): + """DRC-4179: an unmatchable key cannot consume a slot or evict a real dismissal.""" + + def test_an_unmatchable_claude_uuid_is_refused(self) -> None: + config, state = self.runtime() + # Full 36-char UUID for Claude: can never match an 8-character prefix row. + uuid_sid = "12345678-1234-1234-1234-123456789abc" + persisted = dismissals.dismiss(config, state, "claude", uuid_sid, now=1000.0) + self.assertFalse(persisted) + self.assertEqual((), dismissals.load(config)) + + def test_an_unmatchable_entry_in_store_is_dropped_on_load(self) -> None: + config, _state = self.runtime() + self.write_store( + { + "v": 1, + "entries": [ + # Real 8-char Claude sid + {"harness": "claude", "sid": "abcd1234", "at": 100.0, "seen_activity": 100.0}, + # Junk 36-char UUID sid that can never match + { + "harness": "claude", + "sid": "12345678-1234-1234-1234-123456789abc", + "at": 200.0, + "seen_activity": 200.0, + }, + ], + } + ) + loaded = dismissals.load(config) + self.assertEqual(1, len(loaded)) + self.assertEqual("abcd1234", loaded[0]["sid"]) + + if __name__ == "__main__": unittest.main() diff --git a/cargento/skills/cargento/tests/test_focus.py b/cargento/skills/cargento/tests/test_focus.py index 41b252d0..c3f18b6a 100644 --- a/cargento/skills/cargento/tests/test_focus.py +++ b/cargento/skills/cargento/tests/test_focus.py @@ -1021,7 +1021,7 @@ def test_the_pinned_assembly_is_untouched(self) -> None: # would fail on the reader rather than on an injected token. self.assertNotIn(b' None: self.assertEqual((200, 200, 200), (first, status, listed_status)) answer = json.loads(body) self.assertIs(True, answer["persisted"]) - self.assertEqual(1, answer["cleared"]) + self.assertNotIn("cleared", answer) self.assertEqual( [{"harness": "claude", "sid": "abcd1234"}], [{k: v for k, v in row.items() if k != "at"} for row in json.loads(listed)["cleared"]], @@ -656,7 +656,27 @@ def test_a_body_that_names_nothing_is_a_no_op(self) -> None: with self.subTest(body=body): status, answer = self._post(port, body) self.assertEqual(200, status) - self.assertEqual(0, json.loads(answer)["cleared"]) + self.assertNotIn("cleared", json.loads(answer)) + + def test_an_unmatchable_key_returns_persisted_false_and_does_not_persist(self) -> None: + config, state = self._runtime() + with self._serving(cli.build_application(config, state, clock=time.time)) as port: + status, body = self._post( + port, + json.dumps( + { + "harness": "claude", + "sid": "12345678-1234-1234-1234-123456789abc", + } + ).encode(), + ) + listed_status, listed = self._get(port, "/api/cleared") + self.assertEqual(200, status) + self.assertEqual(200, listed_status) + answer = json.loads(body) + self.assertIs(False, answer["persisted"]) + self.assertNotIn("cleared", answer) + self.assertEqual([], json.loads(listed)["cleared"]) def test_the_rollback_switch_answers_503_on_both_routes(self) -> None: # 503, not 404: under `--no-dismiss` the route exists and the store does diff --git a/cargento/skills/cargento/tests/test_next_attention.py b/cargento/skills/cargento/tests/test_next_attention.py index 9093526c..3082b153 100644 --- a/cargento/skills/cargento/tests/test_next_attention.py +++ b/cargento/skills/cargento/tests/test_next_attention.py @@ -2261,8 +2261,8 @@ def model(self, sessions: list[dict[str, Any]]) -> Any: ) ) - def render(self, sessions: list[dict[str, Any]]) -> str: - payload = {"generated": 10_000, "sessions": sessions} + def render(self, sessions: list[dict[str, Any]], **extra: Any) -> str: + payload = {"generated": 10_000, "sessions": sessions, **extra} rendered = self._run_page_js( "\n".join( ( @@ -2410,6 +2410,14 @@ def test_the_visible_coverage_line_counts_no_end_as_none_not_undefined(self) -> self.assertIn("ends observed on 0 sessions", visible) self.assertNotIn("undefined", visible) + def test_the_visible_coverage_line_says_unobservable_under_no_events(self) -> None: + # DRC-4555: under --no-events ends are unobservable, not zero observed. + html = self.render([self.row(sid="quiet-1")], ends_observable=False) + visible = html.split('
None: def test_the_canonical_loader_is_the_released_ui_bundle(self) -> None: page = frontend_page.load_page() - self.assertEqual(911_238, len(page)) + self.assertEqual(911_302, len(page)) self.assertEqual( - "f7cd56b1aa750b7b8980d4e20e1e55792fba13d995de22497bd732eb5c9afa63", + "7c0dbcb341cd1652e60821fde01b3c1080bc58022100bfa9ba25317fefb1c52a", hashlib.sha256(page).hexdigest(), ) diff --git a/cargento/skills/cargento/tests/test_next_observed.py b/cargento/skills/cargento/tests/test_next_observed.py index 7ddf2a16..a43b13ac 100644 --- a/cargento/skills/cargento/tests/test_next_observed.py +++ b/cargento/skills/cargento/tests/test_next_observed.py @@ -152,6 +152,20 @@ def test_every_denominator_comes_from_the_same_thirteen_sessions(self) -> None: self.assertEqual(["alpha", "epsilon", "beta"], out["activeProjects"]) self.assertEqual(["zeta", "delta", "gamma", "theta"], out["rest"]) + def test_gates_coverage_distinguishes_unobservable_ends_from_none_observed(self) -> None: + # DRC-4555: under --no-events ends are unobservable, not zero observed. + out = self._run_page_js( + self.FIXTURE + + """ +payload.ends_observable = false; +console.log(JSON.stringify(nextObserved(payload).coverage.gates)); +""" + ) + self.assertEqual( + "9 of 13 sessions report block state · 4 unknown · ends unobservable", + out, + ) + def test_every_text_has_a_nonempty_value_and_known_boolean(self) -> None: out = self._run_page_js( self.FIXTURE diff --git a/cargento/skills/cargento/tests/test_next_page.py b/cargento/skills/cargento/tests/test_next_page.py index 03e57649..6ea2d483 100644 --- a/cargento/skills/cargento/tests/test_next_page.py +++ b/cargento/skills/cargento/tests/test_next_page.py @@ -616,8 +616,8 @@ def test_load_page_preserves_its_byte_oracles(self) -> None: "b71d627fe8cc06bc4210d23c76c0ee5b2ab644ce15a44c9d617ba66fe398847d", ), "next-observed.js": ( - 31_136, - "931161c99c97d8f72fd30e7c9b104546541979b82eb7c8dec17bb4682d32edf2", + 31_199, + "05d0574cd61663223ee75e8bb9eed8d10729733b737d79a4fa8943793116cb89", ), "next-attention.js": ( 56_558, @@ -664,8 +664,8 @@ def test_load_page_preserves_its_byte_oracles(self) -> None: "62f971c5e2a570068b7e2c3ee72b2499774d14a3b739f6f908962f91b98382f1", ), "next-session.js": ( - 36_503, - "08099b828eb65c1c6c07f214a6e90c1cf18547249f5174e3aaf4379fb77d7bd1", + 36_504, + "4a338ab2c9ce6ae7c197e1361df807a54bbf17b05078541cdba26eeef9fc1c9c", ), "next-workstream.js": ( 18_659, @@ -707,9 +707,9 @@ def test_load_page_preserves_its_byte_oracles(self) -> None: ) assembled = frontend_page.load_page() - self.assertEqual(911_238, len(assembled)) + self.assertEqual(911_302, len(assembled)) self.assertEqual( - "f7cd56b1aa750b7b8980d4e20e1e55792fba13d995de22497bd732eb5c9afa63", + "7c0dbcb341cd1652e60821fde01b3c1080bc58022100bfa9ba25317fefb1c52a", hashlib.sha256(assembled).hexdigest(), ) diff --git a/cargento/skills/cargento/tests/test_next_session.py b/cargento/skills/cargento/tests/test_next_session.py index 0d62b3c8..04e01bb6 100644 --- a/cargento/skills/cargento/tests/test_next_session.py +++ b/cargento/skills/cargento/tests/test_next_session.py @@ -1343,3 +1343,18 @@ def test_a_session_with_no_observed_end_still_reads_idle(self) -> None: self.assertNotIn("session ended", html) self.assertIn("idle", html) + + def test_an_ended_session_does_not_describe_itself_as_awaiting_your_message(self) -> None: + # DRC-4554: header yields present-tense state phrase to an observed end. + html = self.detail( + 'state_detail: "awaiting your message", finished_at: 9350, ended_at: 9400,' + ) + + self.assertIn("ended 10m ago", html) + self.assertNotIn("awaiting your message", html) + + def test_a_non_ended_session_describes_itself_as_awaiting_your_message(self) -> None: + html = self.detail('state_detail: "awaiting your message", finished_at: 9350,') + + self.assertIn("awaiting your message", html) + self.assertNotIn("ended 10m ago", html)