multi-agent-mux/.agents/reports/reviewer-grok-01/report-916abd41.md

Review 916abd41 — PR #59 second pass, Issue #31

Reviewed only the snapshot /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261017T135731Z-e93f0a/1 (loop-20261017T135731Z-e93f0a). Head 2a8d73825b3832376aeaf96b09548359dc3cb887, tree d146f1ac68bcc3485d24fb92edb87264e4d1e8cb, base 26e2c43753169671b600d03274901dbe203988dd is an ancestor. Versus base: 13 files, +1462/−30. Versus the first-review head 6f4f40a4adb2115d5997dffd31fb6473f05cab68: 11 commits (26fb1c4..2a8d738), 7 files, +382/−50. git diff --check is clean. Snapshot porcelain is empty and the snapshot has no .mam. Control-plane main is still 26e2c43753169671b600d03274901dbe203988dd, porcelain empty. No product file was edited.

Host tools: /usr/bin/python3 3.9.6, the control-plane venv the same 3.9.6, /bin/bash 3.2.57 (and /bin/sh is that bash). bash -n accepts stop_session.sh and lib.sh.

Every purge and in-use exercised here used a temporary HOME, CLAUDE_CONFIG_DIR, CLAUDE_PROJECT_DIR, CLAUDE_CODE_TMPDIR, and TMPDIR. The real ~/.claude and /private/tmp/claude-<uid> were listed with read-only counts. _session_file_live was called on the real session files and deleted nothing.

What this Mac already stores

~/.claude has real directories file-history, session-env, sessions, jobs, and projects. file-history holds 19 entries and no symlinks. session-env holds 21 entries and no symlinks. sessions holds 6 *.json files and 6 key files. jobs holds 3 directories, none of them symlinks, and 2 of those names are 8 characters. projects has 16 keys, none of them symlinks.

/tmp is a symlink to /private/tmp. /tmp/claude-<uid> and /private/tmp/claude-<uid> both exist, share one real path, and that directory has 7 entries. CLAUDE_CODE_TMPDIR was unset in this review process before the probes pointed it at a temp directory.

lstart on this host

ps -p <pid> -o lstart= with LC_ALL=C prints five fields. With TZ=UTC0 the string differs from the default-TZ string. calendar.timegm of the UTC string sits 0.367 s behind time.time() for a fresh sleep. time.mktime of the default-TZ string sits the same 0.367 s behind. macOS ps honors TZ=UTC0, so the UTC parse in _session_file_live (claude.py:97-105) is the process start, and a DST fold is the same instant in both readings.

On the six live session files, _session_file_live returned live for all six and stale for none. lstart - startedAt/1000 is between −2.362 s and −0.771 s, so lstart <= startedAt/1000 + 2 holds for every one of them.

A temp-home probe of the same function: a live pid with startedAt set to now, NaN, inf, 10**400, a digit string, or boolean True is live. A dead pid is stale, including a dead pid whose startedAt is NaN (ProcessLookupError returns before the time check, claude.py:78-81). The same live pid with startedAt an hour earlier is stale.

The three findings from the first review

Canonical uuid. _canonical (claude.py:10-18) returns the lowercase spelling only when the whole string is a uuid. _canonical("memory") is None, and an uppercase uuid comes back lowercase. On a temp config, purge_artifacts("memory") returned [], residual was empty, conversation_in_use("memory") was false, and both projects/<key>/memory/MEMORY.md and memory.jsonl stayed. purge_artifacts("....") returned [].

Linked directories. _inside (claude.py:108-122) walks dirname(path) with lstat and rejects a symlink component below the root, then requires the resolved parent to sit under the resolved root. A temp <tmp-root>/<key> symlink to an outside directory stayed a symlink, the outside keep.txt stayed, that linked path was listed by residual_artifacts, and the purged list had no outside path. The real file-history/<uuid> and the jsonl for that same id were removed once the holder was gone. A file-history/<uuid> symlink was unlinked and its target secret.txt stayed. A symlink child inside a real owned <uuid>/ directory was removed with that directory, and the target file stayed.

Non-finite startedAt. The probe above: NaN, inf, and an int too large for float are live while the pid is alive, and a dead pid is stale. That is claude.py:84-95.

Uppercase spelling while a session file names a live pid: conversation_in_use of the uppercase id was true. python -m lib_py.agents purge claude <UPPER> <ws> exited 4 with {"in_use": true, "purged": []}, and in-use exited 4 with What may hold it: on stderr. The jsonl and file-history/<uuid> stayed (__main__.py:146-148).

A process that still has the lowercase uuid in its argv (sh -c 'sleep 60; true' <uuid>, argv0 sh, 57-character args, visible to ps -eo pid=,ppid=,args=) makes conversation_in_use true for the uppercase spelling and for the lowercase spelling (claude.py:354).

purge_artifacts itself has no in-use gate. It skips a live session file (claude.py:323-327) and still removes the other owned paths when called directly. The purge CLI and stop_session.sh:290-298 both ask conversation_in_use first. Stop captures the in-use stderr with 2>&1 >/dev/null, prints the records-preserved sentence, and exits 4. The passthrough raise SystemExit(1) is stop_session.sh:327. The stop SKILL.md exit-4 row (line 65) says the records are preserved and the session itself may already be down. tests/data/skill_code_blocks.json is unchanged since 6f4f40a and still points that passthrough at stop_session.sh:327.

Tests

Pytest used the control-plane venv, PYTHONDONTWRITEBYTECODE=1, -p no:cacheprovider, and --basetemp /tmp/mam916-916abd41-pybase. Inherited HERDR_*, MAM_*, MQTT_*, SKS_*, WORKSPACE_ROOT, DELEGATE_JOB_LOGS_DIR, HOME_DIR, CLAUDE_PROJECT_DIR, CLAUDE_CONFIG_DIR, CLAUDE_CODE_TMPDIR, CODEX_HOME, and PYTHONPATH were dropped. The suite's own fixtures then point HOME at the test tmp.

On the snapshot, 149 tests ran in 39.61 s: 2 failed, 146 passed, 1 skipped. Modules: tests/test_issue31_purge_residual.py, tests/test_a4_adapter_contract.py, tests/test_issue42_skill_blocks.py, tests/test_issue55_agent_homes.py. The skip is test_claude_skills_link_resolves_to_agents_skills (tests/test_issue42_skill_blocks.py:266), because .claude/skills is absent in this worktree. The issue-31 cases for a malformed id including memory, a linked directory, a linked orphan the CLI must report, NaN/10**400/bool startedAt, the UTC lstart mock, _SYSTEM_TMP staying inside the test, and the stop exit-4 text all passed. So did the a4 composite purge and the issue-55 home guard.

Both failures are test_any_spelling_of_the_uuid_is_the_lowercase_session (upper and mixed) at tests/test_issue31_purge_residual.py:318. The asserts above that line passed: the uppercase id was in use through the session file, purge and in-use both exited 4, and the owned files stayed. Line 315 then starts sh -c "sleep 120" <lowercase-uuid>. Bash 3.2 replaces a one-command -c script with that command. ps shows argv0 sleep and an 8-character argument string, and the uuid is gone. conversation_in_use of the uppercase id is false, which is what line 318 asserts against. The sleep 60; true form keeps the uuid and the same call returns true. The check at claude.py:354 sees a command line that still carries the lowercase id. The process this test starts has already dropped it.

The same new tests copied onto an archive of 6f4f40a fail on the pre-fix behavior. That archive is a separate clone. One line, _SYSTEM_TMP = "/tmp", was added there so the new autouse fixture could patch a name 6f4f40a does not define. _tmp_roots in that commit still hardcodes /tmp and does not read the name. purge_artifacts("memory") returned a session-env/memory path (test_issue31_purge_residual.py:285). The linked-dir purge returned removed paths (:351). The CLI asked about a leftover behind a link did not report that residual (:373). NaN, a digit string, and True were stale for a live pid (:539), and the live-holder purge was allowed through (:556). An uppercase id was outside conversation_in_use (:307). 21 failed, 13 passed, in 2.98 s. The real /tmp/claude-<uid> entry count stayed 7.

The in-use gate in stop_session.sh still runs for every agent that has a purge uuid, agy included. That was already true at 6f4f40a.

After the report, snapshot porcelain is still empty, the snapshot still has no .mam, and control-plane porcelain is still empty at 26e2c43. The herdr config mtime is still 1784456394.

[VERDICT: NOT PASS]