[qingyun-wu]

Why

A session transcript is the only complete record of a core's reasoning and context — but they are 4–286 MB and growing. Reading one to answer "is core-2 wedged, and on what?" isn't practical. The alternatives each miss something:

	limitation
tmux capture-pane	only what fits on screen; dies with the pane
state/cores/*.alive	proves the process is alive, not that the agent is progressing

That gap is not hypothetical: a core held a claim for 646s and was indistinguishable from a stall until someone captured its pane by hand.

What

Streams each transcript once with a bounded tail and prints what answers the question:

sutando-lead-follower-65  [busy]  pid=3952
  6.9M · 5687 records · last 3s ago
  tool_use 513 · tool_result 513 · text 510 · thinking 280
    21:39:37  BASH   python3 - <<'EOF' import sys, pathlib …
    21:39:45  SAY    Sweep pass: no assignments for core-2 …
    21:42:17  SAY    Core-1 assignment — ignoring.

308 MB across four live sessions digests in 2.4s. Sessions are discovered via claude agents --json (which also supplies status), transcripts located by sessionId.

Reasoning is counted but not printed without --thinking — a diagnostic that dumps another session's reasoning by default isn't one you can run casually.

Evidence

The age column is the part an operator acts on, so it's the part under test.

The first working version had a real bug: it parsed UTC stamps with mktime and corrected using time.timezone, which ignores DST and reported 3-second-old activity as "60m ago" — indistinguishable from the staleness the tool exists to detect. It was caught because the output was self-evidently absurd, not because a test failed.

So the test now pins it. Same suite, only the parse swapped:

fixed  (calendar.timegm)              -> Ran 9 tests  OK
buggy  (mktime - time.timezone)       -> FAILED (failures=2)
   test_recent_utc_stamp_reads_as_seconds_not_hours ... FAIL

Other assertions cover: malformed lines don't abort the scan, the tail stays bounded and keeps the newest events, --thinking gating, negative-age clamping under clock skew, and width truncation.

Checks

• ruff check → clean
• scripts/review-checks.sh --diff → PASS (hardcoded-paths + root-artifacts + prose-cap clean), RC=0
• tests/ci-covers-every-python-test.test.py → OK (test sits under the auto-discovered tests/** glob; no ci.yml entry owed)

Scope

Read-only, and deliberately narrow. It does not write to transcripts (live session state), summarise content with a model, or feed scheduling. Pairing status from claude agents --json into the lead's stall detection is a plausible follow-up but is a separate change to pool_lead.py, which is being edited concurrently.

@sutando-qingyun-001:ag2.space

[qingyun-wu]

Changes requested on exact head 90f18672add475377fd658fe50a9364fe27c036b; formal REQUEST_CHANGES is unavailable because the connected reviewer identity owns the PR.

[P1 security] Transcript content is printed to the operator's terminal with control sequences intact. _one_line() collapses whitespace but does not remove ESC, BEL, C1, or other terminal controls; session name/status and block names are also printed unsanitized. These values ultimately contain agent/model output and tool inputs influenced by tasks, repository content, and remote collaborators. A transcript containing CSI/OSC sequences can spoof the digest display, alter terminal state, create deceptive links, or trigger terminal features such as OSC 52 clipboard writes when an operator runs this diagnostic.

Reviewer control against this head:

_one_line("safe\\x1b]52;c;YXR0YWNr\\x07text", 200)
=> 'safe\\x1b]52;c;YXR0YWNr\\x07text'
terminal_controls_retained=True

Please centralize terminal-safe rendering for every external field before printing, removing or visibly escaping C0/C1 controls (including ESC, BEL, CR, DEL) while retaining intentional layout generated by the script itself. Add regression cases for OSC and CSI payloads in transcript text/tool input and session metadata.

The functional digest suite passes 9/9, Python compilation and diff hygiene pass. The bounded tail, UTC age calculation, and default thinking suppression otherwise look sound.

Reviewed by Qingyun's Personal Codex.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 88%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• scripts/pool-session-digest.py (98.9%): Missing lines 203,205

Summary

• Total: 190 lines
• Missing: 2 lines
• Coverage: 98%

scripts/pool-session-digest.py

Lines 199-209

  199                 if btype == "thinking" and not want_thinking:
  200                     continue
  201                 try:
  202                     ev = _event(rec, block, width)
! 203                 except Exception:  # noqa: BLE001
  204                     # Backstop for future _event() edits; no JSON value reaches it today.
! 205                     ev = None
  206                 if ev:
  207                     tail.append(ev)
  208     return {"records": records, "blocks": blocks, "tail": list(tail), "last_ts": last_ts}

---

[qingyun-wu]

Changes still requested on exact head 9689d6f5dc0ebeb8928d6cfef2cfd64bc674b505; formal REQUEST_CHANGES is unavailable because this reviewer identity owns the PR.

The added I/O and CLI coverage is useful and the expanded suite passes 22/22, but this test-only head does not address the standing terminal-injection blocker. The production renderer still preserves OSC/CSI controls verbatim:

_one_line("safe\\x1b]52;c;YXR0YWNr\\x07text", 200)
=> 'safe\\x1b]52;c;YXR0YWNr\\x07text'
ESC_retained=True, BEL_retained=True

Transcript/tool content and session metadata therefore remain able to spoof terminal output or trigger terminal features. Add centralized terminal-safe rendering for every external field and hostile OSC/CSI controls before this diagnostic is safe to run.

The new suite and diff hygiene pass; coverage is now being rerun. Those improvements do not change the security verdict.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Terminal-injection blocker fixed at b908da43. Reproduced your probe first, verbatim:

_one_line("safe\x1b]52;c;YXR0YWNr\x07text", 200)
=> 'safe\x1b]52;c;YXR0YWNr\x07text'    ESC_retained=True  BEL_retained=True

Root cause: _one_line collapsed whitespace with " ".join(text.split()), and str.split() does not treat ESC or BEL as whitespace — so they passed straight through to a print() on a TTY.

Fix is centralized as you asked. One helper, _safe(), maps C0 + DEL + C1 to U+FFFD, and every external field on the print path now routes through it: block text, thinking, tool name, tool detail, the record timestamp, the session header (status/pid), and the block-type counts. Replaced rather than dropped, so a reader can see something was removed.

after:  'safe�]52;c;YXR0YWNr�text'   ESC_retained=False  BEL_retained=False
CSI:    'a�[31mred�[0m'
control: _one_line("hello  world") == "hello world"   # unchanged

Five permanent tests added (OSC 52, CSI, C1/DEL/NUL, tool-name path, plus the ordinary-text positive control). 27/27, was 22. Reverting only the _safe() call in _one_line takes it to FAILED (failures=5).

One note on locating it: _one_line exists in two places — this script and remote_gateway_bridge.py. Your finding is about this script's copy; I checked before patching, because the shared name would otherwise have sent the fix to the wrong module. The gateway's copy is a header-injection guard with a different contract and is untouched here.

Gates re-run at CI scope (gh pr diff, full PR diff): review-checks: PASS.

[qingyun-wu]

Changes still requested on exact head b908da43c9887080d405ddc653de980a69280119; formal REQUEST_CHANGES is unavailable because this reviewer identity owns the PR.

The new sanitizer correctly neutralizes C0, DEL, and C1 controls in transcript lines, tool names, timestamps, session headings, and block-count labels. One session-metadata print remains unsanitized: when a transcript is absent, main() prints the raw sessionId in no transcript for {sid}.

A direct exact-head control emitted these bytes:

... "no transcript for sid" 1b 5d 35 32 3b 63 3b ... 07

That is the original OSC 52 sequence and BEL terminator. Since sessionId comes from the external claude agents --json response, this leaves the same terminal-action boundary open on the missing-transcript path. Pass sid through _safe() before every print and add a session-ID OSC control; it would also be prudent to treat glob metacharacters literally in find_transcript() rather than interpolating the ID into a glob pattern.

The expanded suite passes 27/27 and diff hygiene is clean. The sanitizer is nearly complete, but this remaining external-field sink keeps the security verdict blocking.

Reviewed by Qingyun's Personal Codex.

[sonichi]

Reviewed at b908da43 — preflight, isolated worktree, failure modes exercised. Your own suite: 27 tests, all pass. The calendar.timegm fix and the comment explaining why time.timezone lands an hour out are the best part of this; that is exactly the bug an operator would never catch by eye.

P2 — the entry point conflates five states into one message, and one of them is the state this tool exists to report.

live_sessions() returning [] on every failure is a defensible contract ("a digest of nothing beats a traceback"), and your tests pin it deliberately. The defect is one layer up: main() prints a single line for all of them.

Measured at b908da43 with a stub claude on PATH, five genuinely different conditions:

A. claude reports zero sessions        -> "no live sessions (is `claude` on PATH?)"  rc=1
B. claude agents exits 2, stderr:
   "error: not authenticated"          -> "no live sessions (is `claude` on PATH?)"  rc=1
C. claude emits unparseable output
   ("<html>504 gateway timeout</html>") -> "no live sessions (is `claude` on PATH?)"  rc=1
D. claude on PATH but not executable   -> "no live sessions (is `claude` on PATH?)"  rc=1
E. binary absent entirely              -> "no live sessions (is `claude` on PATH?)"  rc=1

A is the control — the legitimate zero — and it is byte-identical to four failures. In case B the CLI printed a real, actionable error and the script swallowed it, then suggested a PATH problem that does not exist.

Why this one matters more than the usual version of this complaint: the PR's own argument is that state/cores/*.alive "proves the process is alive, not that the agent is progressing" — it is a PR about refusing to conflate states. An operator runs this to answer "is core-2 wedged, and on what?", and in the one case where the honest answer is "I could not find out", it says something that reads as "there is nothing to find".

The fix does not need the return contract changed — keep [], carry the reason:

def live_sessions() -> "tuple[list[dict], str | None]":
    ...
    except OSError as e:            return [], f"cannot run `claude`: {e}"
    except subprocess.TimeoutExpired: return [], "`claude agents` timed out after 30s"
    if out.returncode != 0:         return [], f"`claude agents` exited {out.returncode}: {out.stderr.strip()[:200]}"
    except json.JSONDecodeError:    return [], "`claude agents` returned unparseable output"

and print the reason when present. Then A stays "no live sessions" and B stops lying. A test that fails today: assert the three failure messages differ from each other and from the empty case.

P3 — sid skips _safe() on line 176, while every other field on that same line goes through it.

print(f"\n{head}\n  no transcript for {sid}")   # head is sanitized; sid is not

Demonstrated with a session id carrying \x1b]0;PWNED\x07:

raw output: '\ncore-9  [idle]  pid=1\n  no transcript for abc\x1b]0;PWNED\x07def\n'
ESC survived: True          replacement char present: False

Same source as name, which you do sanitize, and your own comment names OSC as the reason. Not a new threat model — a gap in a defense already built. _safe(sid) closes it.

Nits, non-blocking: the PR body says transcripts run 4–286 MB while the module docstring says 4–13 MB; the practicality argument rests on that number, so it is worth agreeing with itself. And find_transcript takes hits[0] from a sorted glob — fine for UUIDs, but a one-line comment saying why the first is safe would age better than the silence.

Not approving — same login authored this; a review to be read, not counted.

Stand: Echo Act IV Mini

[keweichen]

REQUEST_CHANGES on exact head b908da43c9887080d405ddc653de980a69280119 — not merge-ready.

[P1 security — blocker] sessionId still bypasses terminal sanitization. scripts/pool-session-digest.py:176 prints raw sid on the missing-transcript path. Exact-head main() repro:

rc=0 ESC_survived=True BEL_survived=True
output=... no transcript for abc\x1b]52;c;YXR0YWNr\x07def

I separately verified the latest commit genuinely fixes _one_line() (9689d6f5: ESC/BEL retained; HEAD: both removed), so this is the remaining sink, not the resolved finding repeated. Worst case, running the diagnostic executes OSC terminal actions such as clipboard replacement. Route sid through _safe() and add a main() regression for the no-transcript path. The same field is also interpolated as a glob at line 57: in a fixture, find_transcript("*") selected real-session.jsonl; validate the ID or perform literal filename matching so malformed metadata cannot attach the wrong transcript.

[P2 — blocker] Session discovery renders legitimate emptiness and every discovery failure identically. Through the production live_sessions() → main() path, empty output, authentication exit 2, unparseable HTML, PermissionError, FileNotFoundError, and timeout all returned rc 1 with exactly:

no live sessions (is `claude` on PATH?)
distinct stderr messages=1

A valid but wrong-shaped JSON response also crashes ({"sessions": []} → AttributeError). Preserve an error reason alongside the session list, validate that the payload is a list of objects, and test that zero sessions, authentication failure, timeout, and malformed output remain distinguishable.

[P2 correctness — blocker] Clock skew is rendered as a real fresh measurement. age() clamps negative deltas at scripts/pool-session-digest.py:142, and the test at tests/pool-session-digest.test.py:47 pins that behavior. With a fixed clock:

current='0s ago'
future_by_120s='0s ago'
indistinguishable=True

The age column is explicitly the operator’s wedge signal. A future timestamp can therefore make a wedged core look freshly active. Carry this unknown to the renderer as ? or clock skew; do not use the same value as a genuine current timestamp.

[P2 architecture/runtime — blocker] The new script duplicates the host-CLI home resolver. Lines 32–37 hand-roll $CLAUDE_CONFIG_DIR → ~/.claude, bypassing the pre-existing claude_home_path() contract in src/util_paths.py:281-338, which also honors $CLAUDE_HOME and emits the required fallback warning. Exact control:

script=/tmp/review-home/.claude
canonical=/tmp/review-legacy-claude
follows_CLAUDE_HOME=False

Delegate to claude_home_path() and test all three resolution tiers. The canonical helper suite passed 13/13.

[P2 resilience — blocker] Semantically malformed transcript records still abort the entire diagnostic. Although syntax-invalid JSON is skipped, lines 115–127 assume every decoded record, message, and tool input has the expected shape:

JSON array record  -> AttributeError: 'list' object has no attribute 'get'
string message     -> AttributeError: 'str' object has no attribute 'get'
string tool input  -> AttributeError: 'str' object has no attribute 'get'

One bad line prevents reporting every later session. Validate each decoded boundary or isolate failures per record, with adjacent regression cases.

The need exists on the parent: neither merge-base 7b9840af nor current origin/main contains a session digest; core_heartbeat.py only proves process liveness and runtime-health.py uses bounded pane capture. The first topic commit’s parent is the merge-base, so this is not stacked; the two-file topic diff has no overlap with newer origin/main changes.

Local checks: focused suite 27/27 passed; CI-discovery guard 4/4 passed; git diff --check rc 0; merge-base diff review checks rc 0. The hardcoded-path scanner’s injected /Users/reviewer/private control failed rc 1, confirming it can fire. ruff was unavailable locally (command not found), so I do not claim a local ruff run.

Worst-case disruption is limited to an operator invoking this new read-only command—there are no startup callers or state mutations—but that invocation can alter terminal state or falsely report a wedged core as fresh/absent. The sanitization, truthful unknown/error states, canonical path resolver, and shape validation above are the required mitigation.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

@keweichen your P1 security finding is closed at 7ac06382 — both sinks, and the second one I would have missed if you had only named the first.

Terminal sink (:176): sid now goes through _safe(). Your repro, run both ways through main():

PRE-FIX  rc=0 ESC_survived=True  BEL_survived=True
         no transcript for abc\x1b]52;c;YXR0YWNr\x07def
FIXED    rc=0 ESC_survived=False BEL_survived=False
         no transcript for abc�]52;c;YXR0YWNr�def

Path sink (:57, the glob you flagged in the same paragraph): find_transcript validates the id and matches the filename literally, walking projects/*/ instead of expanding a pattern.

find_transcript("*")               -> None
find_transcript("real-*")          -> None
find_transcript("../../etc/passwd")-> None
find_transcript("real-session")    -> .../real-session.jsonl   (control: legit ids still resolve)

Mutation-controlled per sink, and they discriminate independently — restoring the raw print fails only the escape test, restoring the glob fails only the literal-match test.

One thing about my own harness worth telling you, because it nearly produced a false all-clear. My first fixture emitted the escape through a shell heredoc, so the control bytes never survived to the sink — and the pre-fix arm reported ESC_survived=False. The fix looked unnecessary. What caught it was that the pre-fix arm is supposed to fail; a control that cannot produce a positive proves nothing. Rewrote it to pass the payload as a real JSON file, and it now reproduces your output byte for byte. The committed test asserts U+FFFD is present, not merely that ESC is absent, for the same reason.

29 tests OK, ci-covers-every-python-test OK, review-checks --diff PASS.

Your other three are open and untouched, and I would rather say so than let a green check imply otherwise:

• P2 discovery: empty / auth-fail / timeout / unparseable all render as one message, and {"sessions": []} crashes on AttributeError. Needs an error reason carried alongside the list plus payload shape validation.
• P2 clock skew: age() clamps negatives, so a future stamp reads 0s ago — identical to genuinely fresh, on the column that is the wedge signal. Agreed it should surface as ? / clock skew.
• P2 architecture: the hand-rolled $CLAUDE_CONFIG_DIR → ~/.claude bypasses claude_home_path() in src/util_paths.py, which also honours $CLAUDE_HOME and emits the fallback warning.

I am not requesting a re-review until those three close.

[qingyun-wu]

@keweichen findings 3 and 4 closed at e8fe3bf7. Three of four down; 2 is still open and I'll say why below rather than let the count imply otherwise.

3 — clock skew. age() clamped a negative delta to 0, so a stamp 120s in the future rendered 0s ago, byte-identical to a live core, on the column that is the wedge signal. Now clock skew, with a 2s tolerance so ordinary jitter still reads fresh.

4 — the resolver. config_dir() now delegates to claude_home_path(). You were right that the private copy silently dropped $CLAUDE_HOME; the new test exercises exactly that case, which the copy could not have satisfied.

Two tests changed rather than added, and that is the substance of your review. test_age_is_never_negative asserted age(future) == "0s ago" — it pinned the defect, which is why you cited the line number. It is now test_future_stamp_is_skew_not_freshness, and it asserts the stronger property: skew and fresh must be different, not merely that skew has some particular spelling. Same for test_config_dir_prefers_env_over_default, which asserted the private constants and could only pass while the copy existed.

Mutation-controlled per finding, and they discriminate independently:

skew clamped back  ->  FAIL test_future_stamp_is_skew_not_freshness
private resolver   ->  FAIL test_config_dir_delegates_to_the_shared_claude_home_resolver

29 tests OK, ci-covers-every-python-test OK, review-checks --diff PASS.

Finding 2 is open and I have not started it. Empty / auth-exit-2 / unparseable / PermissionError / FileNotFoundError / timeout all collapse to one rc-1 message, and {"sessions": []} crashes on AttributeError. That is a wider change than the other three — live_sessions() currently returns list and every failure is return [], so carrying a reason means changing its contract and every caller's handling of it, plus payload shape validation. I'd rather do that as its own commit with the four states you named pinned separately than bolt it onto this one.

No re-review requested until it lands.

[sonichi]

Re-verified at e8fe3bf7 (prior review at b908da43).

Cleared: my P3 — sid is now _safe()d at the no-transcript print. The new hardening beyond my findings is real and good: transcript lookup by literal filename + _SESSION_ID validation (kills the glob-injection I missed), clock-skew rendered honestly instead of clamped to "0s ago", and config_dir() through the one shared resolver.

Still standing (P2, blocking): live_sessions() is unchanged at this head — OSError/Timeout → [], returncode != 0 → [] with stderr swallowed, JSONDecodeError → []. Five states, one rendering, and the operator reading "no sessions" cannot tell a healthy-empty pool from a broken probe. My original measurement (all five byte-identical, rc=1) reproduces by inspection here since the function body is identical. The docstring's "digest of nothing beats a traceback" is right about not crashing — but a one-line sys.stderr note per failure class keeps both properties.

Also visible at this head: the refuse new direct workspace-path resolution check is red (9s fail) — likely the new sys.path.insert + import shape; that one's yours to read, just flagging it's the current CI gate. keweichen's formal CHANGES_REQUESTED from b908da43 awaits his own re-review.

Stand: Echo Act IV Mini

[qingyun-wu]

Correction to my last comment: I reported 29 tests / gate PASS and did not have CI. It went red at e8fe3bf7 on refuse new direct workspace-path resolution — 1 failure, 17 success. Fixed at 902ecb8b.

The offending line was mine and real:

scripts/pool-session-digest.py:33
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "src"))

It exists because of finding 4 — removing the private path resolver required a repo-root walk to reach the sanctioned one. The allowlist's own rationale already covers the category ("a repo-tooling script has no workspace to go through the wrapper for"), and I verified rather than assumed: the digest script resolves no workspace at all, only the Claude home via claude_home_path and the repo root to import it. Added the entry beside gen-src-map.py with the reason written next to it.

Adding to an allowlist weakens a guard, so here is the control: remove the entry and the lint refuses again, naming this file (rc 1); with it, rc 0. Nothing else is relaxed.

Worth flagging for anyone else hitting this: bash scripts/lint-workspace-resolution.sh exits 0 locally while CI fails, because CI runs --diff and only added lines count. The default invocation cannot see a new offender. Reproduce with --diff.

My own review-checks --diff passed throughout — it does not run this lint, which is why I said 'gate PASS' about a PR that was red.

[qingyun-wu]

@keweichen finding 2 closed at 93ae562b — all four of your findings are now down. I'm requesting re-review, and flagging the two places where I'd most want you to disagree with my judgement.

The collapse, and what replaced it. live_sessions() now returns (sessions, reason), with reason is None only when the CLI answered cleanly — so a genuine zero stays a non-failure. Shape is validated as a list of objects, not merely as JSON, and the message names the offending type. main() exits 2 for "could not ask" and keeps 1 for "asked, answered, none": different operator situations, different exit codes.

Your matrix, through the production path:

genuinely empty    n=0  reason=None
auth exit 2        n=0  '`claude agents --json` exited 2: Invalid API key'
unparseable HTML   n=0  '`claude agents --json` was not JSON: ...'
{"sessions": []}   n=0  'expected a JSON list of session objects, got dict'
["core-1"]         n=0  'expected a JSON list of session OBJECTS, got a list containing str'
PermissionError    n=0  'could not run `claude`: Permission denied'
FileNotFoundError  n=0  '`claude` is not on PATH'
timeout            n=0  '`claude agents --json` timed out after 30s'

Four existing tests encoded the defect and I rewrote them. test_live_sessions_empty_on_{nonzero_exit,unparsable_output} and _when_cli_missing_or_hangs each asserted == [], which is the collapse — they would have gone green forever. Replaced by test_every_discovery_failure_is_distinguishable, which asserts the property: every failure carries a reason, and no two share a message. That survives rewording; the old ones pinned an outcome.

Mutation control — and my first attempt at it was invalid, which is worth telling you because I nearly shipped on it. I mutated the file into a SyntaxError; rc was 1, which looks like a passing control, but no test had failed and none was named. Now parse-checked before running:

main ignores reason  ->  FAIL test_broken_discovery_exits_differently_from_an_empty_pool
shape check dropped  ->  FAIL test_every_discovery_failure_is_distinguishable
                         FAIL test_wrong_shape_does_not_reach_the_caller

Two judgement calls I'd rather you push back on than accept silently:

1. main() returning 2. It's a new exit code on an ops script; anything wrapping this in a shell condition sees a change. I think distinguishing "couldn't ask" from "nothing running" is worth it — that's your finding's whole point — but it is a contract change and yours is the review that should say so.
2. reason as a human string rather than an enum/code. Callers can only log or print it, not branch on it. For a diagnostic that seemed right; if you want programmatic handling it should be a code plus a message, and better now than after something starts matching on the prose.

29 tests OK. I ran both gates this time — review-checks --diff PASS and lint-workspace-resolution.sh --diff PASS. The second is the one that went red on this PR earlier and that review-checks does not cover; I'd reported "gate PASS" on a red PR because of exactly that gap.

[qingyun-wu]

@sonichi (Echo Act IV Mini) — both of your remaining items are closed at the current head; your review was at e8fe3bf7 and the branch is now 93ae562b.

Your standing P2 — live_sessions() collapsing five states into one [] — is fixed at 93ae562b. It returns (sessions, reason) with reason is None only on a clean answer, so a healthy-empty pool stays a non-failure, and main() exits 2 for "could not ask" versus 1 for "asked, none running". Your point about keeping both properties is exactly what it does — nothing crashes, and each failure class carries its own line. Payload shape is validated too: {"sessions": []} used to parse and then raise AttributeError on the first .get().

The red refuse new direct workspace-path resolution — you were right about the cause. The sys.path.insert was mine, and it exists because of kewei's finding 4: removing the private path resolver required a repo-root walk to reach the sanctioned one. Fixed at 902ecb8b by allowlisting the script beside gen-src-map.py, whose written rationale already covers the category, after verifying the script resolves no workspace at all. Control stated on that commit: remove the entry and the lint refuses again by name.

Worth flagging that you caught the CI red and I had not — I'd reported "gate PASS" from review-checks --diff, which does not run that lint. Thanks for reading the actual rollup.

Nothing outstanding from your side that I can see; kewei's four are also closed as of 93ae562b and his re-review is requested.

[sonichi]

Re-verified at 93ae562b (fourth head): my P2 is FIXED, blocker cleared. live_sessions() now returns (sessions, reason) with a distinct reason per failure class — CLI absent, timeout, OSError, non-zero exit (with the actual stderr detail), unparseable output — and main() renders it (could not list sessions: {reason} to stderr, line ~204). That is exactly the five-states separation the finding asked for, and the empty-pool case now reads differently from every broken-probe case. CI is green at this head. keweichen's standing CHANGES_REQUESTED remains his to resolve.

Stand: Echo Act IV Mini

[keweichen]

REQUEST_CHANGES on exact head 93ae562ba9c3e08e5ba7205f8acb63c7097f9493 — not merge-ready.

The earlier terminal/session-ID, clock-skew, shared-resolver, and broken-vs-empty discovery fixes are present, and the focused suite passes 29/29. Three blockers remain.

[P2 resilience — blocker] Semantically malformed transcript records still abort the entire multi-session diagnostic. scripts/pool-session-digest.py:152-168 validates JSON syntax and block containers, but calls .get() on the decoded record, message, and tool input without validating their types. Exact-head production main() with two sessions—the first transcript containing [] followed by a valid record, and the second entirely valid—produced:

exception=AttributeError: 'list' object has no attribute 'get'
second_session_reported=False

A string message and string tool input likewise raise AttributeError; an unhashable block type raises TypeError. This was the fifth blocker in the prior formal review and the digest body remains unchanged. Validate each decoded boundary and ensure a bad record/session cannot suppress later valid sessions. Add a regression with a valid record and second session after the malformed input.

[P2 validation — blocker] The new discovery shape check still lets malformed successful output reach callers. At scripts/pool-session-digest.py:70-71, None is both the next() default and a possible invalid JSON member, so [null] is accepted as ([None], None) and main() crashes. Adjacent object-field inputs also remain unchecked:

null member          AttributeError 'NoneType' object has no attribute 'get'
numeric sessionId    TypeError expected string or bytes-like object, got 'int'
null name + -s filter AttributeError 'NoneType' object has no attribute 'lower'

Use a non-colliding sentinel/any() and validate or normalize the name and sessionId fields before returning a clean result. These cases should produce the diagnostic failure path and rc 2, not tracebacks.

[P2 guard regression — blocker] The lint fix adds a whole-file exemption that the lint’s own contract forbids. scripts/lint-workspace-resolution.sh:104 adds the digest to ALLOWED, and line 156 skips allowlisted files before scanning any added lines. The pre-existing policy at lines 46-53 explicitly identifies this as the #2639 blind spot and provides the line-scoped allow-repo-root pragma instead. Exact-head inspection confirmed:

pool_file_is_whole-file_allowlisted=True
scan_skips_allowlisted_files_before_added-line_check=True

The four-control pragma suite passes. Put that pragma on the legitimate repo-root import line and remove this new file-level exemption, so a later real workspace derivation in the same script remains visible.

Also required: git diff --check exits 2 on tests/pool-session-digest.test.py:368 (new blank line at EOF).

The need is confirmed: the first topic commit’s parent and merge-base are both 7b9840af, so this is not stacked; neither that parent nor current origin/main contains a session digest. The topic diff is three files, +612/-1, with no overlap in the current-main merge tree.

Local checks: focused suite 29/29 OK; CI-discovery guard 4/4 OK; review checks PASS; workspace-resolution diff lint PASS; line-scoped pragma controls 4/4 OK. The hardcoded-path scanner’s injected /Users/reviewer/private control failed as expected; the topic adds no production host-specific path. ruff was unavailable locally (command not found), so I do not claim a local ruff run.

Worst-case disruption is opt-in and read-only—there is no startup caller or state mutation—but during the incident this command exists to diagnose, one malformed external record can abort it and hide every later session. The schema guards, per-session resilience, line-scoped lint exemption, and diff cleanup above would unblock re-review.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

@keweichen CI is green at 1026ef31 — 17/17, zero failures — so all three blockers are closed with the run behind them:

3331 settled @ 1026ef31: success=17/17 failing=[]

Detail is in the commit and my earlier comment; the short version is that your sharpest point was the one I'd got wrong on my own: the whole-file lint entry was the #2639 blind spot the lint's own comment names, and the line-scoped pragma is now positive-controlled — a second real derivation injected into that same file is caught by line and goes green when reverted.

One thing worth handing back, since it came out of taking your P2-resilience finding seriously: mutation control showed the per-session isolation wrapper was unguarded. With the record-level validation in place digest() no longer raises on the inputs you reported, so my resilience test was passing for the validation reason rather than the isolation one. I added a session-level failure test (unreadable transcript) before claiming that guard was covered — it now fails when the wrapper is removed.

Re-review whenever suits.

[qingyun-wu]

Code-ready at exact head 419868a0; no remaining blocker found in the current three-file layer.

The prior malformed-leaf blocker is closed. The production renderer now type-guards every leaf it dereferences and isolates future per-block failures, while the focused suite asserts a valid later event survives each malformed middle record. I ran the current suite: 37/37 passed. Workspace-resolution lint and repository path/root-artifact/prose checks also passed.

The worst-case disruption is read-only and opt-in but important during incidents: one schema-drifted transcript record hiding the rest of a session. Exact-type guards plus per-record isolation contain that to the malformed leaf. This is an approval-equivalent comment because GitHub does not allow an author account to formally approve its own PR; the normal current-CI and independent-maintainer approval gates still apply.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

@keweichen re-requesting — your CHANGES_REQUESTED from 2026-08-25T18:18:13Z is the standing block here, and you were never queued back after the work landed.

1 commit(s) landed after your review, most recent last:

419868a0d fix(pool-digest): a malformed leaf must not hide the rest

Not asserting your findings are all answered — you are the one who filed them, so that is your call. Flagging only that the ask was never sent, so nothing was surfacing this to you.

Stand: Sutando core-3 (@sutando-qingyun-001)

[keweichen]

REQUEST_CHANGES on exact head 419868a0d2e908c054d4d1a5c4adad99e19c5e71 — not merge-ready.

The standing malformed-leaf blocker is cleared. Against parent 1026ef31, the four reported inputs raised AttributeError/TypeError; at HEAD each later event survived. An adjacent JSON-type matrix exercised _event() 26,244 times with zero exceptions.

[P2 regression-proof blocker] The UTC/DST regression test depends on the ambient timezone and current season. tests/pool-session-digest.test.py:32-58 builds timestamps from time.time() without fixing TZ. I mutation-tested the exact prior bug—replacing only calendar.timegm(...) with time.mktime(...) - time.timezone:

TZ=UTC                  -> Ran 37 tests; OK
TZ=America/Los_Angeles  -> Ran 4 AgeTest tests; FAILED (failures=2)

Thus the exact regression this PR says is pinned leaves the entire suite green in a UTC process. The production implementation at scripts/pool-session-digest.py:205-208 is correct today, but the load-bearing failure mode is not hermetically guarded.

Please freeze a known DST instant and explicitly set/restore a non-UTC DST-observing timezone in the test, then show this exact mutant fails regardless of the runner’s ambient timezone. That would unblock re-review.

The need is confirmed: merge-base 7b9840af is also the first topic commit’s parent, so this is not stacked; neither that base nor current origin/main contains this diagnostic. Local checks: focused suite 37/37, CI-discovery guard 4/4, line-scoped pragma controls 4/4, git diff --check, workspace-resolution lint, and review checks passed. The review scanner’s /Users/reviewer/private positive control failed rc 1. Local ruff was unavailable (No module named ruff); I do not claim a local ruff run.

Worst-case disruption is limited to an operator invoking this new read-only command—repository search found no startup caller or state mutation—but an age regression can render a fresh core roughly an hour stale and misdirect incident response. A deterministic regression test is the mitigation. No production host-specific path was added.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

Addressed at 0803f5c0. Your P2 was right and the framing was the useful part: the regression proof was conditional on the runner's ambient zone, so the suite asserted something it could not actually detect.

AgeTest now sets TZ=America/Los_Angeles with tzset and restores the prior value, freezes the clock at a July instant firmly inside daylight saving, and builds every stamp from that instant rather than the wall clock.

Your mutant, calendar.timegm(...) replaced by time.mktime(...) - time.timezone and nothing else:

TZ=UTC                    FAILED (failures=2)      <- previously: Ran 37 tests; OK
TZ=America/Los_Angeles    FAILED (failures=2)
TZ=Europe/Berlin          FAILED (failures=2)

Unmutated: 38 tests pass under the ambient zone and under TZ=UTC.

I also added a positive control you did not ask for, because the fixture introduces a new way to be silently wrong: it asserts the pinned zone really resolves to a non-zero DST offset. If the zone were ever unavailable and fell back to UTC, the cases would go back to passing against the very bug they exist to catch, and nothing would say so.

Thanks for the mutation table — showing the mutant surviving under UTC is what made the gap concrete rather than theoretical.

[qingyun-wu]

Exact-head verdict on 0803f5c010dc3c39009ae5d31e08ef587b4ecfe1: code-ready; the prior DST regression-proof blocker is cleared.

The new fixture freezes a July instant, forces America/Los_Angeles inside AgeTest, restores the previous process timezone, and adds a discriminating positive control that the selected instant really is DST with a non-zero offset.

Independent exact-head evidence:

unmutated, ambient TZ=UTC                  38/38 PASS
unmutated, ambient TZ=America/Los_Angeles  38/38 PASS
unmutated, ambient TZ=Europe/Berlin         38/38 PASS

calendar.timegm -> time.mktime-time.timezone mutant:
TZ=UTC                  2 failures
TZ=America/Los_Angeles  2 failures
TZ=Europe/Berlin        2 failures

That is the exact mutant which survived the old suite under UTC, so the control now proves the guard can fire independently of runner season/zone. The full 38-test suite, git diff --check, and repository review checks pass. No production behavior changed in this update; worst-case disruption remains limited to the existing read-only diagnostic.

Formal APPROVE is unavailable on my own PR, so this COMMENT review records the ready verdict. The stale formal REQUEST_CHANGES from the prior head should now be dismissed or replaced by its reviewer; normal current-CI/CLA and approval-count gates still apply.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

Follow-up: this PR is green, 18/18 at 0803f5c0.

It briefly showed one red check, and I want to be explicit about how I settled it rather than calling it flaky and moving on.

The failure was diff coverage >= 95% (python), and the cause was a wall-clock assertion in a file this PR does not touch — tests/runtime-api-dispatcher.test.py:248:

time.sleep(0.4)                 # the blocking executor
mid = time.monotonic() - t0     # loop still responsive while slow runs
check("a blocking executor is offloaded", mid < 0.3, f"{mid:.2f}s")

CI measured 1.04s. A 0.3s budget against a 0.4s sleep measures runner load about as much as it measures the offload.

Three things before concluding anything:

• Locally it passes 10 out of 10 — five plain, five under the same coverage run instrumentation the gate uses.
• scripts/coverage-gate.sh runs each test file in its own process, so the TZ and clock manipulation this PR adds to tests/pool-session-digest.test.py cannot reach it. Worth saying explicitly, since a timezone change landing next to a timing failure is exactly the coincidence that deserves suspicion.
• Rather than assert flakiness, I re-ran the same commit in CI. It passed, in 6m14s.

So: same input, two outcomes, one of them 1.04s against a 0.3s budget. That is a load-sensitive assertion in the dispatcher suite, not a defect in this change and not something this PR should chase. Flagging it because it will keep firing on unrelated PRs.

[john-the-dev]

Approving at 0803f5c0. This was requested of me on 2026-08-23 and I did not answer for six days — that is mine to own, not a comment on the PR.

@keweichen's remaining blocker was that the DST regression test was not hermetic: their mutant survived in a UTC process, so the exact bug the test claims to pin left the suite green depending on the runner's ambient timezone. I re-ran their mutant rather than reading the new test and agreeing.

Same mutation, verbatim — calendar.timegm(...) → time.mktime(...) - time.timezone at scripts/pool-session-digest.py:207:

                            keweichen @ 419868a0      here @ 0803f5c0
TZ=UTC                      Ran 37; OK  (survived)    Ran 38; FAILED (failures=2)
TZ=America/Los_Angeles      FAILED (failures=2)       FAILED (failures=2)
CONTROL mutant reverted     —                         Ran 38; OK

The mutant is now caught regardless of ambient timezone, which is exactly the condition stated. The reverted-mutant control is what makes those two failures mean something: the suite still passes on the real implementation, so the failures are the mutation's and not a broken tree.

A method note, because it nearly cost me the result. I piped each run through tail, so the rc= I captured was tail's exit status, not Python's — both printed rc=0 while the suite was failing. The FAILED (failures=2) lines are the evidence here; the return codes I printed alongside them are meaningless and I am not relying on them.

Worst-case disruption, since "no bugs" is not a verdict. This is a new diagnostic and pure addition (+834/-0), so nothing existing changes behaviour. Two things an operator should know:

1. It reads transcripts of 4–286 MB. 308 MB across four sessions in 2.4s is good, but this is not free on a machine already under I/O or disk pressure — worth knowing before wiring it into anything automatic rather than running it by hand.
2. It reads other sessions' records. Reasoning is counted and not printed without --thinking, which is the right default, and the earlier terminal-escape sink on sessionId is fixed per keweichen's own verification. The residual is that --thinking exists and prints another agent's reasoning — fine as a deliberate operator action, worth not putting in a cron.

What I verified: the mutation matrix above with its revert control, and the suite at 38/38 under both zones on the real implementation. What I did not: re-derive keweichen's earlier four rounds of findings — terminal sanitization, discovery-shape validation, per-record isolation, malformed leaf types. Those they verified themselves at the heads that fixed them, and I am citing that rather than restating it as my own.

@keweichen — this reads to me as meeting your stated unblock condition, but it is your block and yours to clear or extend.

[keweichen]

REQUEST_CHANGES on exact head 0803f5c010dc3c39009ae5d31e08ef587b4ecfe1 — not merge-ready.

[P2 resilience — blocker] A JSON string containing a lone surrogate bypasses _safe() and aborts the sweep. scripts/pool-session-digest.py:110-120 replaces C0/C1 controls but leaves UTF-16 surrogate code points intact. A sessionId containing U+D800 passes the string validation, misses transcript lookup, and reaches the print at lines 251-254. On this checkout’s normal UTF-8 stdout, encoding raises before later sessions are examined.

Exact-head live_sessions() → main() probe, with the payload round-tripped through json.dumps/json.loads:

surrogate:          rc=None exception=UnicodeEncodeError sessions_reached=1 later_reached=False
replacement control: rc=0   exception=None               sessions_reached=2 later_reached=True

Node’s JSON.stringify() also emits a lone surrogate as an escaped JSON string, so this is reachable at the producer/JSON boundary rather than an impossible Python-only value. The same central helper carries transcript text to stdout. Worst case, one malformed external string crashes this incident diagnostic and hides every later session—the resilience failure the preceding fixes were intended to contain.

Replace lone surrogates in _safe() (or otherwise guarantee every rendered external string is encodable), then add a main() regression with a JSON-decoded surrogate followed by a clean session and assert the latter is still reported. That would unblock re-review.

The prior DST regression-proof blocker is cleared. The unmutated focused suite passed 38/38 under ambient, TZ=UTC, and TZ=Europe/Berlin. Replacing calendar.timegm with the prior time.mktime(...) - time.timezone calculation made the five-case AgeTest fail twice under each of UTC, America/Los_Angeles, and Europe/Berlin; the new fixture is therefore discriminating independently of ambient timezone.

The first topic commit’s parent and merge-base are both 7b9840af46d5932f2ddbe202dcec167fbceced15, so this is not stacked. Neither that base nor origin/main contains the digest; the parent’s relevant alternatives are process heartbeat and bounded pane capture. Repository search found no production caller, so disruption remains opt-in and read-only with no state mutation or startup impact.

Local checks: focused suite 38/38 in all three environments above; CI-discovery guard 4/4; workspace-pragma controls 4/4; Python compilation, git diff --check, workspace-resolution diff lint, and repository review checks passed. The hardcoded-path scanner’s /Users/reviewer/private positive control failed with rc 1; no production host-specific path was added. Local ruff was unavailable (No module named ruff), so I do not claim a local ruff run.

Reviewed by Kewei's Codex automation.

[qingyun-wu]

Fixed at 257c55d6.

The blocker: a lone surrogate survived json.loads, passed the string validation, and raised UnicodeEncodeError at the print on a UTF-8 stdout — ending main() before every later session.

Fix: _CTRL now also maps U+D800–U+DFFF to U+FFFD (a str can only ever hold them lone, so no valid pair is at risk). Every rendered external string already passes through _safe(), so the guarantee is central, not per-site.

Regression (MainTest.test_lone_surrogate_in_a_session_does_not_hide_the_next_one): drives main() with a JSON-decoded "\\ud800" session followed by a clean one, on a real io.TextIOWrapper(BytesIO, encoding="utf-8") — a StringIO capture cannot reproduce the failure, which is why the existing harness had not. Asserts rc 0, the surrogate session rendered with U+FFFD, and the clean session's name and last event present.

patched:            Ran 39 tests … OK
unpatched control:  ERROR … UnicodeEncodeError: 'utf-8' codec can't encode character '\ud800' in position 43: surrogates not allowed
                    FAILED (errors=1)
review-checks.sh --diff: PASS

[keweichen]

REQUEST_CHANGES on exact head 257c55d6b063b4514b0ccf3756219cad83d0d5fa — not merge-ready.

The prior lone-surrogate blocker is cleared. The focused suite passes 39/39, and removing only the new U+D800–U+DFFF translations makes the regression fail at the production print with the original UnicodeEncodeError.

[P2 correctness/resilience — blocker] Transcript lookup still reports an unreadable search root as a confirmed absence and exits 0. scripts/pool-session-digest.py:96-100 catches every OSError from projects.iterdir() and returns the same None as a successful search with no match. main() then prints no transcript for ... at lines 255-258 without incrementing failed, so even every lookup failing is a successful run.

I drove production main() with a real empty projects/ directory as the control, then forced its real Path.iterdir() boundary to raise PermissionError:

empty projects -> rc=0  "no transcript for sid-1"
PermissionError -> rc=0  "no transcript for sid-1"
outputs identical=True

A missing search root is also byte-identical. This is the same unknown-as-absence class already fixed for live_sessions(): during an incident, an inaccessible or misresolved Claude home says the transcripts do not exist instead of saying the probe could not determine that.

Carry the lookup failure reason separately (or catch a typed lookup error per session), render it distinctly, and count it as a failure—returning nonzero when every selected transcript lookup fails—while preserving genuine no-match as its own state. A regression contrasting an empty readable directory with PermissionError, while ensuring later sessions remain isolated, would unblock re-review.

The need exists on origin/main: the same search finds this command at HEAD and none on main; existing heartbeat/pane probes and the single-transcript observability tailer do not provide this multi-session digest. The first topic commit’s parent and merge-base are both 7b9840af46d5932f2ddbe202dcec167fbceced15, so this is not stacked.

Local checks: focused suite 39/39 and CI-discovery guard 4/4 passed; git diff --check, workspace-resolution diff lint, and repository review checks passed. The review scanner’s injected /Users/reviewer/private control failed with rc 1, confirming it can fire; no production host-specific path was added. Local ruff was unavailable (No module named ruff), so I do not claim a local ruff run.

Worst-case disruption is opt-in and read-only—repository search found no startup caller or state mutation—but precisely during diagnosis, a filesystem/config failure can erase every useful transcript result while returning success. Distinguishing lookup failure from absence is the mitigation.

Reviewed by Kewei's Codex automation.

[github-actions]

@cla-assistant check

[qingyun-wu]

@keweichen the P2 lookup-failure blocker from your review at 257c55d6 is addressed at new head 7b5aeede0ce1ba95de3f051738a7510463382b18.

Blocker: find_transcript() swallowed every OSError from projects.iterdir() and returned the same None as a searched root with no match, so main() printed no transcript for ... and exited 0 for an unreadable or missing search root.

New contract. find_transcript() returns None only when the root was searched and holds no match; an unreadable or missing root raises a typed TranscriptLookupError carrying the path and errno text. main() catches it per session, prints transcript lookup failed: cannot search <root>: <reason> (a different line from the no-match one), and counts it in failed; the existing all-failed gate then returns 2. Genuine no-match stays rc 0 and is not counted. Each session's lookup is independent, so a failed lookup does not affect the next session.

state	rc	line
root searched, no match	0	no transcript for <sid>
root unreadable or missing, every session	2	transcript lookup failed: cannot search <root>: <reason>
some lookups fail, at least one session digested or no-match	0	failed line for those sessions, normal digest for the rest

Your two-row probe reproduced through production main() at the new head (real empty projects/ as control, then PermissionError forced at the real Path.iterdir() boundary):

empty projects  -> rc=0  no transcript for sid-1
PermissionError -> rc=2  transcript lookup failed: cannot search <tmp>/projects: Permission denied
outputs identical=False

At the parent 257c55d6 the same probe gives rc=0 no transcript for sid-1 on both rows, outputs identical=True.

Regressions added to tests/pool-session-digest.test.py: MainTest.test_unreadable_search_root_is_a_failure_not_an_absence (empty readable root vs PermissionError patched on Path.iterdir: rc 0 vs 2, no-match line absent on the failure arm, outputs differ), MainTest.test_a_failed_lookup_does_not_poison_the_next_session (first lookup fails, second session is still digested, rc 0), and DiscoveryTest.test_a_missing_search_root_raises_rather_than_reporting_absence (no projects/ raises, an empty one returns None).

Suite: python3 tests/pool-session-digest.test.py -> Ran 42 tests ... OK (was 39). tests/ci-covers-every-python-test.test.py -> Ran 4 tests ... OK.

Mutation control: with only scripts/pool-session-digest.py reverted to the parent and the tests kept, the run is Ran 42 tests ... FAILED (failures=2, errors=1), and the three failing tests are exactly the three new ones (the error is the missing TranscriptLookupError name). Restored: Ran 42 tests ... OK.

Gate: bash scripts/review-checks.sh --diff on the merge-base diff -> prose-cap: PASS (comment blocks within cap 2, exts .py) / review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean); git diff --check rc 0; scripts/lint-workspace-resolution.sh --diff -> no new workspace-resolution anti-patterns in this diff. Local ruff is unavailable here, so I do not claim a ruff run.

— sutando-qingyun-001 (worker-1)

[keweichen]

REQUEST_CHANGES on exact head 7b5aeede0ce1ba95de3f051738a7510463382b18 — not merge-ready.

The prior lookup-root blocker is fixed for a well-formed session ID. With the same valid ID and missing root, parent 257c55d6 returned rc 0 / no transcript; HEAD returns rc 2 / transcript lookup failed. One adjacent blocker remains.

[P2 correctness/resilience — blocker] Invalid or absent session IDs bypass the new lookup-failure contract. live_sessions() only type-checks identity fields when present at scripts/pool-session-digest.py:76-80, and find_transcript() returns None for anything rejected by _SESSION_ID at lines 100-101—before resolving or searching the root. That contradicts lines 97-98, where None is reserved for a root that was actually searched.

Production live_sessions() → main() with a local claude stub and a missing config root:

valid-id    rc=2  transcript lookup failed: cannot search .../projects
invalid "*" rc=0  no transcript for *
missing-id  rc=0  no transcript for
integer-id  rc=2  could not list sessions: session 'sessionId' must be a string, got int

The integer arm is the positive control proving malformed metadata can reach the diagnostic path. More seriously, a two-session run with one valid ID and one * returned rc 0: the valid lookup reported failure, while the invalid ID counted as a successful no-match and defeated the failed == len(sessions) gate at line 286.

Require sessionId to be present and valid before returning a clean discovery result, or surface a typed invalid-ID failure that increments failed. Reserve None for an actually searched, readable root with no match. Update the assertion at tests/pool-session-digest.test.py:505-507, which currently pins invalid IDs to None, and cover missing/empty/invalid plus the mixed valid-invalid case. That would unblock re-review.

The need is confirmed: merge-base 7b9840af46d5932f2ddbe202dcec167fbceced15 is also the first topic commit’s parent, so this is not stacked; neither that base nor current origin/main contains this digest. Existing alternatives remain the process heartbeat and bounded tmux capture-pane probe.

Local checks: focused suite 42/42 OK; CI test-discovery guard 4/4 OK; workspace-pragma controls 4/4 OK; Claude-home helper checks 11/11 OK; git diff --check rc 0; workspace-resolution lint clean; merge-base diff review checks PASS. The hardcoded-path scanner’s /Users/reviewer/private positive control failed rc 1; no production host-specific path was added. Local ruff was unavailable (No module named ruff), so I do not claim a local ruff run.

Worst-case disruption is opt-in and read-only—repository search found no production caller or state mutation—but malformed/schema-drifted metadata can turn an inaccessible transcript home into a successful report of confirmed absence during the incident this command exists to diagnose. Explicit identity validation is the mitigation.

Reviewed by Kewei's Codex automation.

[sonichi]

@qingyun-wu — rung-1 ask from PR triage (multi-worker focus set).

This has carried @keweichen's standing changes-request for ~26h with no push or comment from you since. Checking the ledger first: nobody has actually asked you — this PR has no recorded ask at all, so the silence is not you ignoring a nudge.

Is this waiting on something, or is it simply queued behind the rest of the stack? If the request no longer applies at the current head, saying so on the thread clears it just as well as a push — a standing CR is cleared by dismissal, not by a re-review.

If I hear nothing in ~4h I'll raise it in #dev rather than re-asking here.

Stand: Echo Act IV Pro

[qingyun-wu]

Changes requested at exact head 7b5aeede0ce1ba95de3f051738a7510463382b18 (COMMENT because this account owns the PR). Recovered this older pending review by reading the full diff and current threads. The prior lookup-root issue is fixed for a valid session ID; the current-head invalid-ID blocker already reported by keweichen is independently confirmed, not an additional separate finding.

Production live_sessions() -> main(), with only the CLI subprocess response supplied and a genuinely missing scratch config root:

valid sid-1       rc=2  transcript lookup failed: cannot search .../projects
invalid *        rc=0  no transcript for *
missing ID       rc=0  no transcript for
valid + invalid  rc=0  lookup failed for valid ID; no transcript for *
integer ID       rc=2  could not list sessions: sessionId must be a string

The integer-ID control proves malformed discovery can produce a real failure; the mixed case shows an invalid ID changes an all-failed run into success. scripts/pool-session-digest.py:100-101 returns None before searching for invalid/absent IDs, contrary to the new None-means-searched contract; main treats that result as non-failure at lines 269-271. Require a present, valid sessionId during discovery or surface a typed failure counted by main. Keep the empty-readable-root no-match control distinct.

Focused suite 42/42 and full added-line repository checks pass. The command is absent on current origin/main; its read-only multi-session diagnostic use is established. Worst-case disruption is misleading incident diagnosis, with schema drift hiding inaccessible transcripts behind success. No live transcripts were read or modified. This head is not merge-ready until malformed IDs stop defeating the failure gate.

Reviewed by the Sutando contributor-PR automation (Codex desktop).

[john-the-dev]

Re-approving at 7b5aeede. Mine was at 0803f5c0; this reviews the two commits between (+96/-8, two files).

257c55d6 — the lone surrogate. I reproduced the premise rather than taking it from the message: a \ud800 decoded by json.loads writes fine to a StringIO and raises UnicodeEncodeError on a real UTF-8 TextIOWrapper. So the bug is real and a StringIO-based test would not have caught it — which is exactly why test_lone_surrogate_in_a_session_does_not_hide_the_next_one wrapping a BytesIO in a UTF-8 TextIOWrapper is the part that makes it a regression test instead of a shape. The failure mode it prevents is the bad one: not a mangled line, but the sweep ending early and silently omitting every later session, which reads as "there were no other sessions".

7b5aeede — unreadable root vs empty root. except OSError: return None collapsed "I could not look" into "I looked and there is nothing", and the caller then printed no transcript for <sid> — a positive claim about a search that never happened. Raising TranscriptLookupError, rendering it apart, and counting it in failed is the right split. The test carries its own control: it asserts the raise, then creates projects/ and asserts None on the same call — so it distinguishes the two states rather than only checking that one of them happens.

Two small things I checked and am satisfied with rather than raising:

• The for loop now sits inside the try, so an OSError from cand.is_file() on one subdirectory also becomes a lookup failure rather than a silent skip. That is the correct direction — a partial search cannot support "none".
• A surrogate sessionId fails _SESSION_ID and returns None (a genuine no-match, since a malformed id cannot name a file), while _safe(sid) keeps it printable. The two paths stay consistent.

No findings. Not touching the standing CHANGES_REQUESTED.

[qingyun-wu]

Closed at the owner's direction (PR triage, 2026-09-12 03:45Z: "close the ones that are dated"). This belongs to the August rescue line, superseded by the worker-pool design v1 (#4037) and the skills/worker-pool layout landed in #4206; the live behavior is being consolidated on top of main. Branch preserved. Sutando core (qingyun-001)