docs(issue-42): review - attach names the herdr server, --dry-run is not "write nothing", a deleted block stays deleted
Merge-workspace review of PR #46 (codex R1-R3 and C1, claude O1 and O4). - R1: resume never attaches, and the server it exports does not reach the calling shell; the native herdr binary does not route on environment variables. The attach advice and the Verification block now name the server: `herdr --session <server> agent attach <name>`, with <server> the row's herdr_session. - R2: --dry-run in resume and create skips the spawn and the registry update, but the checks before it still run and on macOS remove the quarantine attribute from the agent binary. The option rows say so. - R3: test_no_deleted_body_came_back compares every current block with the deleted bodies by exact sha and by a whitespace-insensitive fingerprint (stored as base_fingerprint), independently of the inventory, so a deleted block cannot return by being listed again under class c. Replaying the review's probe (the deleted resume Workflow re-added and inventoried as c) fails the new test and passes the old one. - C1 / O1: the base create call block at 263-266 (class c) folded into the Workflow call is recorded under "superseded"; the 37 base blocks are all accounted for. - O4: create exit 42 also covers MAM_KILL_ON_READY_TIMEOUT=1; stop's no-op exit 0 does not apply with --purge-conversation. - The inventory's positions and shas are regenerated for the edited blocks; every classification is unchanged. No product code changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
This commit is contained in:
1 parent
8d26106195
commit
17eba0904e
5 files changed
+93
-19
No files matched your search
@@ -339,9 +339,9 @@
|
||||
{
|
||||
"skill": "multi-agent-mux-resume",
|
||||
"start": 103,
|
||||
"end": 124,
|
||||
"end": 128,
|
||||
"lang": "bash",
|
||||
"sha": "f4bddf5ba4d1",
|
||||
"sha": "8a468dfa43bd",
|
||||
"class": "b",
|
||||
"kind": "verification",
|
||||
"evidence": [],
|
||||
@@ -492,7 +492,8 @@
|
||||
],
|
||||
"replacement": "bash .agents/skills/multi-agent-mux-resume/scripts/resume_session.sh --workspace <path> --agent <agent> --session <name>",
|
||||
"script": "multi-agent-mux-resume/scripts/resume_session.sh",
|
||||
"note": "Workflow re-wrote resume_session.sh in bash"
|
||||
"note": "Workflow re-wrote resume_session.sh in bash",
|
||||
"base_fingerprint": "cd4000a4afae"
|
||||
},
|
||||
{
|
||||
"skill": "multi-agent-mux-stop",
|
||||
@@ -511,7 +512,8 @@
|
||||
],
|
||||
"replacement": "bash .agents/skills/multi-agent-mux-stop/scripts/stop_session.sh",
|
||||
"script": "multi-agent-mux-stop/scripts/stop_session.sh",
|
||||
"note": "pre-flight (registered? already stopped?); replaced by one prose line, the Workflow block already calls the entry point"
|
||||
"note": "pre-flight (registered? already stopped?); replaced by one prose line, the Workflow block already calls the entry point",
|
||||
"base_fingerprint": "f2e5380dbaa4"
|
||||
},
|
||||
{
|
||||
"skill": "multi-agent-mux-create",
|
||||
@@ -531,7 +533,8 @@
|
||||
],
|
||||
"replacement": "bash .agents/skills/multi-agent-mux-create/scripts/create_session.sh --workspace <path> --agent <agent> --role <role>",
|
||||
"script": "multi-agent-mux-create/scripts/create_session.sh",
|
||||
"note": "pre-flight; replaced by prose, the has_real_herdr vs command -v pitfall kept as prose"
|
||||
"note": "pre-flight; replaced by prose, the has_real_herdr vs command -v pitfall kept as prose",
|
||||
"base_fingerprint": "6caf3573a587"
|
||||
},
|
||||
{
|
||||
"skill": "multi-agent-mux-create",
|
||||
@@ -554,7 +557,8 @@
|
||||
],
|
||||
"replacement": "bash .agents/skills/multi-agent-mux-create/scripts/create_session.sh --workspace <path> --agent <agent> --role <role>",
|
||||
"script": "multi-agent-mux-create/scripts/create_session.sh",
|
||||
"note": "Workflow re-wrote create_session.sh in bash"
|
||||
"note": "Workflow re-wrote create_session.sh in bash",
|
||||
"base_fingerprint": "bc8eec474b36"
|
||||
},
|
||||
{
|
||||
"skill": "multi-agent-mux-create",
|
||||
@@ -569,7 +573,8 @@
|
||||
"mismatch": [],
|
||||
"replacement": "bash .agents/skills/multi-agent-mux-create/scripts/create_session.sh --workspace <path> --agent <agent> --role <role>",
|
||||
"script": "multi-agent-mux-create/scripts/create_session.sh",
|
||||
"note": "manual \"append this row\" step; the script writes the row. The lib.sh shim note (base 248-251) moved to prose under Output format"
|
||||
"note": "manual \"append this row\" step; the script writes the row. The lib.sh shim note (base 248-251) moved to prose under Output format",
|
||||
"base_fingerprint": "b8ff0badf537"
|
||||
}
|
||||
],
|
||||
"entry_points": {
|
||||
@@ -620,5 +625,19 @@
|
||||
"C6: create_session.sh header lists exit 4 \"YAML append failure\"; there is no literal `exit 4`, but the YAML write raises SystemExit(4) for a duplicate running row (create_session.sh:515) and it passes through, so 4 IS a real exit code (the plan assumed it was not)",
|
||||
"R6: resume's \"The script above re-passes them\" and \"as done in Workflow above\" pointed at the removed block; reworded (same for create's Verification note)",
|
||||
"R7 (review 08e1f1b2): stop's table said exit 1 is always pre-stop; the registry update after the kill chain also raises SystemExit(1) (stop_session.sh:310) when the row disappeared"
|
||||
],
|
||||
"superseded": [
|
||||
{
|
||||
"skill": "multi-agent-mux-create",
|
||||
"base_start": 263,
|
||||
"base_end": 266,
|
||||
"lang": "bash",
|
||||
"base_sha": "ceb96a8c1293",
|
||||
"class": "c",
|
||||
"kind": "invocation",
|
||||
"replacement": "bash .agents/skills/multi-agent-mux-create/scripts/create_session.sh",
|
||||
"note": "The base call example under 'Updating agent-sessions.yaml' (with the nonexistent agent-sessions-yaml-edit script above it, prose_mismatches C4) is folded into the Workflow call block; no information lost.",
|
||||
"base_fingerprint": "b6a8a25279c5"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -4,8 +4,10 @@ tests/data/skill_code_blocks.json lists every fenced block of the eight
|
||||
SKILL.md files with its class - (b) a procedure no script offers yet, (c) an
|
||||
example, call or diagram - and the (a) blocks removed because the entry script
|
||||
already does them. These tests fail when a SKILL.md block changes without the
|
||||
inventory, when an (a) block comes back, or when an exit-code table drifts from
|
||||
the entry script.
|
||||
inventory, when a deleted block's body comes back (by exact sha or by a
|
||||
whitespace-insensitive fingerprint, whatever the inventory says about it), or
|
||||
when an exit-code table drifts from the entry script. A (c) block folded into
|
||||
another call is recorded under "superseded".
|
||||
"""
|
||||
import hashlib
|
||||
import json
|
||||
@@ -127,6 +129,55 @@ def test_no_duplicate_block_left():
|
||||
assert r["class"] == "a", r
|
||||
|
||||
|
||||
def fingerprint(body):
|
||||
"""A deleted body's identity that survives reindenting and reflowing."""
|
||||
return hashlib.sha256(" ".join(body.split()).encode()).hexdigest()[:12]
|
||||
|
||||
|
||||
def _returned_bodies(texts, gone):
|
||||
"""(skill, start, which) for each current block whose body is a deleted one."""
|
||||
hits = []
|
||||
for skill, text in texts.items():
|
||||
for start, _end, _lang, sha, body in parse_blocks(text):
|
||||
for g in gone:
|
||||
if sha == g["base_sha"] or fingerprint(body) == g["base_fingerprint"]:
|
||||
hits.append((skill, start, f"{g['skill']}:{g['base_start']}"))
|
||||
return hits
|
||||
|
||||
|
||||
# 2b. A deleted body stays deleted - even when the inventory is updated to list
|
||||
# it again under another class (review R3).
|
||||
def test_no_deleted_body_came_back():
|
||||
gone = DATA["removed"] + DATA["superseded"]
|
||||
texts = {s: skill_text(s) for s in SKILL_NAMES}
|
||||
assert _returned_bodies(texts, gone) == []
|
||||
|
||||
|
||||
def test_a_returned_body_is_caught_reindented_and_with_the_inventory_updated():
|
||||
"""The guard itself: a current block posing as a deleted one is found by its
|
||||
exact sha and, reindented, by its fingerprint; listing it in the inventory
|
||||
does not hide it (that check never reads DATA["blocks"])."""
|
||||
skill = "multi-agent-mux-stop"
|
||||
start, _end, _lang, sha, body = parse_blocks(skill_text(skill))[0]
|
||||
fake = {"skill": skill, "base_start": 1, "base_sha": sha, "base_fingerprint": fingerprint(body)}
|
||||
assert _returned_bodies({skill: skill_text(skill)}, [fake]) == [(skill, start, f"{skill}:1")]
|
||||
reindented = "\n".join(" " + line for line in body.split("\n"))
|
||||
other = {"skill": skill, "base_start": 1, "base_sha": "0" * 12, "base_fingerprint": fingerprint(reindented)}
|
||||
assert _returned_bodies({skill: skill_text(skill)}, [other]) == [(skill, start, f"{skill}:1")]
|
||||
|
||||
|
||||
# 2c. A deleted (c) block folded into another call is recorded too (review C1):
|
||||
# with it the 37 base blocks are all accounted for.
|
||||
def test_superseded_blocks_are_recorded_and_replaced():
|
||||
assert DATA["superseded"], "the folded create call is recorded"
|
||||
for r in DATA["superseded"]:
|
||||
assert r["class"] == "c" and r["kind"] in KINDS, r
|
||||
bodies = [b[4] for b in parse_blocks(skill_text(r["skill"]))]
|
||||
assert any(r["replacement"] in body for body in bodies), r
|
||||
for r in DATA["removed"] + DATA["superseded"]:
|
||||
assert re.fullmatch(r"[0-9a-f]{12}", r["base_fingerprint"]), r
|
||||
|
||||
|
||||
# 3. A removed block is replaced by the entry-point call.
|
||||
@pytest.mark.parametrize("removed", DATA["removed"], ids=lambda r: f"{r['skill']}:{r['base_start']}")
|
||||
def test_removed_block_replaced_by_entry_call(removed):
|
||||
|
||||
Reference in new issue
Block a user