fix(issue-42): review R1 - stop's exit 1 also comes after the stop
The registry update after the kill chain raises SystemExit(1) when the row disappeared during the run (stop_session.sh:310), so "exit 1 changed nothing" was false on that path. The stop table now says so, the code is declared as a passthrough, and the test requires every integer SystemExit in an entry script's heredocs to be declared - the check that would have caught it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
7670b41afd
commit
e6a46cd7c4
3 files changed
+29
-11
No files matched your search
@@ -59,13 +59,13 @@ bash .agents/skills/multi-agent-mux-stop/scripts/stop_session.sh \
|
|||||||
| Exit | Meaning |
|
| Exit | Meaning |
|
||||||
|---|---|
|
|---|---|
|
||||||
| 0 | Stopped; or already `stopped` (no-op) |
|
| 0 | Stopped; or already `stopped` (no-op) |
|
||||||
| 1 | YAML not found, or the session is not registered |
|
| 1 | From the checks before the stop, nothing was changed: YAML not found, or the session is not registered. **The registry update after the stop also exits 1** (the row disappeared from the YAML during the run) |
|
||||||
| 2 | Bad arguments (including the retired `--mode` / `--capture-id` / `--graceful`), or `--agent` omitted and not resolvable |
|
| 2 | Bad arguments (including the retired `--mode` / `--capture-id` / `--graceful`), or `--agent` omitted and not resolvable |
|
||||||
| 3 | `--purge-conversation` without `--yes` |
|
| 3 | `--purge-conversation` without `--yes` |
|
||||||
| 4 | Purge aborted: the herdr session survived the kill chain |
|
| 4 | Purge aborted: the herdr session survived the kill chain |
|
||||||
| other | The registry update after the stop (YAML / DB write) failed and its code is passed through |
|
| other | The registry update after the stop (YAML / DB write) failed and its code is passed through |
|
||||||
|
|
||||||
Exits 1, 2 and 3 come from checks before anything is stopped, so nothing was changed. On an `other` exit the session may already be down while its row still says `running`: check with `multi-agent-mux-status`, then run the stop again (it is idempotent).
|
Exits 2 and 3, and exit 1 from the pre-stop checks, come before anything is stopped, so nothing was changed. On exit 1 from the registry update, or an `other` exit, the session may already be down while its row still says `running`: check with `multi-agent-mux-status`, then run the stop again (it is idempotent).
|
||||||
|
|
||||||
**Idempotency**: if the row is already `status: stopped`, the script prints `already stopped (...)` and exits 0 — re-running is a safe no-op.
|
**Idempotency**: if the row is already `status: stopped`, the script prints `already stopped (...)` and exits 0 — re-running is a safe no-op.
|
||||||
|
|
||||||
|
|||||||
@@ -593,7 +593,9 @@
|
|||||||
"4",
|
"4",
|
||||||
"other"
|
"other"
|
||||||
],
|
],
|
||||||
"passthrough": {}
|
"passthrough": {
|
||||||
|
"1": "multi-agent-mux-stop/scripts/stop_session.sh:310"
|
||||||
|
}
|
||||||
},
|
},
|
||||||
"multi-agent-mux-create/scripts/create_session.sh": {
|
"multi-agent-mux-create/scripts/create_session.sh": {
|
||||||
"skill": "multi-agent-mux-create",
|
"skill": "multi-agent-mux-create",
|
||||||
@@ -616,6 +618,7 @@
|
|||||||
"C4: create named a nonexistent `agent-sessions-yaml-edit` script; removed (create_session.sh writes the row)",
|
"C4: create named a nonexistent `agent-sessions-yaml-edit` script; removed (create_session.sh writes the row)",
|
||||||
"C5: create's cmd_full table (claude = `claude`) disagreed with adapter spawn-spec; replaced by \"decided by spawn-spec\"",
|
"C5: create's cmd_full table (claude = `claude`) disagreed with adapter spawn-spec; replaced by \"decided by spawn-spec\"",
|
||||||
"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)",
|
"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)"
|
"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"
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
@@ -54,22 +54,34 @@ _HEREDOC = re.compile(r"(?<!<)<<-?\s*(['\"]?)(\w+)\1")
|
|||||||
_EXIT = re.compile(r"(?<![\w.])exit\s+(\d+)\b")
|
_EXIT = re.compile(r"(?<![\w.])exit\s+(\d+)\b")
|
||||||
|
|
||||||
|
|
||||||
def literal_exit_codes(script):
|
_RAISE = re.compile(r"SystemExit\((\d+)\)")
|
||||||
"""Codes the entry script exits with literally: `exit N` in shell context.
|
|
||||||
|
|
||||||
Only the entry file itself (not lib.sh or sub-scripts); heredoc bodies and
|
|
||||||
comment lines are skipped, and `sys.exit(N)` never matches."""
|
def _scan_exits(script):
|
||||||
codes, pending = set(), []
|
"""(`exit N` codes in shell context, {code: line} of `SystemExit(N)` in heredoc bodies).
|
||||||
for line in (SKILLS / script).read_text(encoding="utf-8").split("\n"):
|
|
||||||
|
Only the entry file itself (not lib.sh or sub-scripts). Comment lines are
|
||||||
|
skipped, and `sys.exit(N)` never matches `exit N`. A heredoc's SystemExit is
|
||||||
|
the exit code of the unguarded Python step under set -e."""
|
||||||
|
codes, raised, pending = set(), {}, []
|
||||||
|
for n, line in enumerate((SKILLS / script).read_text(encoding="utf-8").split("\n"), 1):
|
||||||
if pending:
|
if pending:
|
||||||
if line.strip() == pending[0]:
|
if line.strip() == pending[0]:
|
||||||
pending.pop(0)
|
pending.pop(0)
|
||||||
|
else:
|
||||||
|
for code in _RAISE.findall(line):
|
||||||
|
raised.setdefault(code, n)
|
||||||
continue
|
continue
|
||||||
if re.match(r"^\s*#", line):
|
if re.match(r"^\s*#", line):
|
||||||
continue
|
continue
|
||||||
codes.update(_EXIT.findall(line))
|
codes.update(_EXIT.findall(line))
|
||||||
pending.extend(m.group(2) for m in _HEREDOC.finditer(line))
|
pending.extend(m.group(2) for m in _HEREDOC.finditer(line))
|
||||||
return codes
|
return codes, raised
|
||||||
|
|
||||||
|
|
||||||
|
def literal_exit_codes(script):
|
||||||
|
"""Codes the entry script exits with literally: `exit N` in shell context."""
|
||||||
|
return _scan_exits(script)[0]
|
||||||
|
|
||||||
|
|
||||||
def exit_table(skill):
|
def exit_table(skill):
|
||||||
@@ -137,6 +149,9 @@ def test_exit_table_agrees_with_script(script):
|
|||||||
literal = literal_exit_codes(script)
|
literal = literal_exit_codes(script)
|
||||||
assert literal <= numeric, f"{script} exits with {sorted(literal - numeric)} but the table lacks them"
|
assert literal <= numeric, f"{script} exits with {sorted(literal - numeric)} but the table lacks them"
|
||||||
passthrough = ep["passthrough"]
|
passthrough = ep["passthrough"]
|
||||||
|
raised = _scan_exits(script)[1]
|
||||||
|
assert {c: f"{script}:{n}" for c, n in raised.items()} == passthrough, \
|
||||||
|
f"{script}: SystemExit codes in heredocs {raised} must be declared as passthrough (and in the table)"
|
||||||
assert numeric <= literal | set(passthrough), \
|
assert numeric <= literal | set(passthrough), \
|
||||||
f"table codes {sorted(numeric - literal - set(passthrough))} come from nowhere in {script}"
|
f"table codes {sorted(numeric - literal - set(passthrough))} come from nowhere in {script}"
|
||||||
for code, ref in passthrough.items():
|
for code, ref in passthrough.items():
|
||||||
|
|||||||
Reference in new issue
Block a user