GodopuandClaude Opus 5.5
ffe8665b27
Merge review on macOS (--review-only loops, --all-reviewer): round 1 at320aa9f4/4 NOT PASS, round 2 codex F1 (a claimed dry-run job holds its session), round 3 4/4 PASS (discarded: the worktree changed mid-review), round 4 at21f0c12claude/agy/grok PASS with codex interrupted by a stream disconnect, round 5 codex PASS. Full suite at21f0c12on macOS / Python 3.9 / bash 3.2 / herdr 0.9.3: 1860 passed / 0 failed / 38 skipped; no new entries in ~/.claude/projects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
37 lines
6.0 KiB
Markdown
37 lines
6.0 KiB
Markdown
# Review 51507ec8 — the monitor labels a row with the server that answered
|
||
|
||
Head `21f0c12b9e2195cba8e196d6ccc2469173302e8a` (tree `c2357ac01824d099fcbb7e9f7433b6b05f5e86a6`) is the one commit `21f0c12 fix(issue-50): the monitor labels a row with the server that answered` on base `efd27def47a4d67684694ccf43a6ebeede01620f`. Two files, +9/−1. `git diff --check` is clean. Snapshot porcelain empty. Control-plane main is still `42e51735ba285930fc5b8ee70ba343277e9b1adb`, porcelain empty.
|
||
|
||
Drift B writes the row after `pane_meta` and the cwd check. When the scan key is `default` and `HERDR_SESSION_NAME` is set to something else, that name replaces `srv` before `server_opt` and the row are built (`reconcile.sh:664-666`). `herdr_session`, `herdr_server`, and the start, attach, and kill commands all use that name (`:681-687`). The scan (`:505-509`), `pane_meta` (`:540-544`), and drift A (`:594-595`) still use the scan key.
|
||
|
||
## Why the label matches the server that answered
|
||
|
||
`unique_servers` always contains `default` and, when the variable is set, `HERDR_SESSION_NAME` (`reconcile.sh:494-496`). A `default` scan runs `herdr ls` with no `-L`. The shim's `ls` reads `HERDR_SESSION_NAME` (`lib.sh:1256-1259`) and calls `_real_herdr agent list` (`:1274`). `_real_herdr` prepends `--session` whenever that value is non-empty (`:270-283`; `default` is treated as empty at `:272-274`). A create exports `HERDR_SESSION_NAME` (`create_session.sh:373`) and its end-of-run `reconcile.sh --once` is a child of that process (`:748`), so the inherited value is the create's server.
|
||
|
||
On this Mac, herdr 0.9.3, throwaway home `/tmp/m515h2`, session `t515b`: the server socket appeared under that home. `herdr --session t515b agent list` exited 0. `herdr agent list` with no `--session` exited 1 with `server_not_running` for `/tmp/m515h2/.config/herdr/herdr.sock`. The generated shim's `ls`, with `HERDR_SESSION_NAME=t515b`, no `-L`, and `MAM_NO_AUTOSTART=1`, exited 0. The same `ls` with the variable unset exited 1 and printed `no server running (herdr session default)`. `workspace create` on that server returned a pane; `agent list` stayed an empty agent list, so the shim's `ls` printed no name. The exit codes are the routing check. No socket named `t515b` appeared under the user home.
|
||
|
||
Create adopts a monitor row when `herdr_server or herdr_session or 'default'` equals its own `HERDR_SESSION_NAME` (`create_session.sh:522`, `:540-543`). After this label they are the same string, the epoch check is unchanged, and the row is replaced. A row that fails that test still raises `SystemExit(4)` at `create_session.sh:550`. `tests/data/skill_code_blocks.json` now points exit code 4 at that line.
|
||
|
||
## Side effects
|
||
|
||
A throwaway workspace under `/tmp/m515ws` ran this snapshot's `reconcile.sh --once`. The shim was the one `lib.sh` writes, and the binary it called was a stand-in that returns one pane. `HERDR_SESSION_NAME=t515probe`.
|
||
|
||
- Both scans called `agent list --session t515probe`. `default` sorts first, so the first call is the `default` scan. The result listed the pane under both scan keys, `probe-creator-claude|default` and `probe-creator-claude|t515probe`. The yaml received one row, `status: running`, `herdr_server` and `herdr_session` `t515probe`, note `auto-registered by monitor`, kill command `herdr -L t515probe kill-session -t probe-creator-claude`. `pane_meta` ran once. The second sighting is skipped because the name is added to `yaml_session_names` after the append (`reconcile.sh:715`) and the next entry hits the name check (`:610-611`).
|
||
- A second `--once` with `HERDR_SESSION_NAME` unset scanned the real default (the stand-in was asked for `agent list` with no `--session`) and the row's server (`--session t515probe`). The live set was `probe-creator-claude|t515probe`. The row stayed `running` on `t515probe`. Drift A did not fire. Drift A compares `(name, herdr_session or herdr_server or 'default')` with the scan keys (`:594-595`), and the daemon's server set includes each row's `herdr_session` (`:499-502`).
|
||
- With no `HERDR_SESSION_NAME`, a new pane visible only on the real default scan was stored as `herdr_server: default` and the kill command has no `-L`. The new `if` does not run.
|
||
- A row already stored as `default` stayed `running`, stayed `default`, and kept its note. The new `if` is on the new-entry path, past the name skip.
|
||
|
||
A create's own `--once` still puts `default` in the scan set, and that scan is still answered from the inherited `HERDR_SESSION_NAME`. A row whose server is the real default is compared with that redirected scan for that one pass. The commit leaves the scan and drift A as they were. The row this commit writes is looked up on the following daemon pass by the server name stored on it.
|
||
|
||
## What was run
|
||
|
||
On this Mac, with the pr-56-fix venv (Python 3.9.6), `PYTHONDONTWRITEBYTECODE=1`, `-p no:cacheprovider`, `--basetemp` under `/tmp/m515bt-s4`, `/tmp/m515bt-lock`, and `/tmp/m515bt-skill`. Inherited `HERDR_*`, `MAM_*`, `MQTT_*`, `WORKSPACE_ROOT`, `CLAUDE_PROJECT_DIR`, `HOME_DIR`, and `CODEX_HOME` were unset.
|
||
|
||
- `tests/test_tier4_e2e.py::test_e2e_scenario4_parallel_flock_locking`: 1 passed in 43.01s. Six parallel creates in one sandbox all exited 0, and the yaml held those six names. The mock herdr drops `-L` and `--session`, so this is the regression the full suite hit at `a9ffeda` (rc 4, "already running"). The shim routing above is the separate check that a no-`-L` call reaches `HERDR_SESSION_NAME`.
|
||
- `tests/test_issue50_pane_lock.py`: 11 passed in 54.30s. That includes the adoption of a monitor row of this server and the refusal of another server's row.
|
||
- `tests/test_issue42_skill_blocks.py::test_exit_table_agrees_with_script`: 3 passed in 0.05s. The passthrough for exit code 4 resolves to `create_session.sh:550`, and that line raises `SystemExit(4)`.
|
||
|
||
User `~/.config/herdr/config.toml` mtime stayed `1784456394`. The herdr `sessions` listing hash stayed `0e4d261c4faae218c1e6ec7f2914c0c766df456e0c5e460cb28e2dcb20385177`. The control-plane loop marker mtime stayed `1791769101`. The throwaway server was stopped and its socket removed.
|
||
|
||
[VERDICT: PASS]
|