diff --git a/lane-handoff b/lane-handoff index e1a43cee..ac97303a 100755 --- a/lane-handoff +++ b/lane-handoff @@ -127,6 +127,38 @@ say() { printf '%s\n' "$*"; } note() { printf '%s: %s\n' "$prog" "$*" >&2; } step() { printf '%s: %s\n' "$prog" "$*" >&2; } die() { printf '\n%s: REFUSED: %s\n' "$prog" "$1" >&2; exit "${2:-2}"; } +check_claude_env_dual() { # + local cce_suffix="$1" cce_new_set cce_legacy_set cce_new cce_legacy + case "$cce_suffix" in + WINDOW_REF) + cce_new_set="${OPENBENCHES_CLAUDE_WINDOW_REF+set}" + cce_legacy_set="${WORKBENCHES_CLAUDE_WINDOW_REF+set}" + cce_new="${OPENBENCHES_CLAUDE_WINDOW_REF-}" + cce_legacy="${WORKBENCHES_CLAUDE_WINDOW_REF-}" + ;; + WINDOW_ID) + cce_new_set="${OPENBENCHES_CLAUDE_WINDOW_ID+set}" + cce_legacy_set="${WORKBENCHES_CLAUDE_WINDOW_ID+set}" + cce_new="${OPENBENCHES_CLAUDE_WINDOW_ID-}" + cce_legacy="${WORKBENCHES_CLAUDE_WINDOW_ID-}" + ;; + LANE_DIR) + cce_new_set="${OPENBENCHES_CLAUDE_LANE_DIR+set}" + cce_legacy_set="${WORKBENCHES_CLAUDE_LANE_DIR+set}" + cce_new="${OPENBENCHES_CLAUDE_LANE_DIR-}" + cce_legacy="${WORKBENCHES_CLAUDE_LANE_DIR-}" + ;; + *) + printf 'REFUSED: unknown Claude environment suffix: %s\n' "$cce_suffix" >&2 + return 2 + ;; + esac + if [ "$cce_new_set" = set ] && [ "$cce_legacy_set" = set ] && [ "$cce_new" != "$cce_legacy" ]; then + printf 'REFUSED: OPENBENCHES_CLAUDE_%s and WORKBENCHES_CLAUDE_%s have conflicting values. Set both to the same value, or unset the incorrect one, then re-run. Nothing was renamed or written.\n' \ + "$cce_suffix" "$cce_suffix" >&2 + return 2 + fi +} # Exact launch facts must not carry record delimiters, on Bash 3.2 too. restart_fact_ok() { local LC_ALL=C @@ -1580,6 +1612,11 @@ lifecycle_finish() { # # ------------------------------------------------- 1. the identity triple # +# Ambiguous old/new launcher values refuse before identity selection or any write. +check_claude_env_dual WINDOW_REF || exit +check_claude_env_dual WINDOW_ID || exit +check_claude_env_dual LANE_DIR || exit +# # THE WORKSTATION COMES FROM CONFIGURATION, NEVER FROM `hostname` (Amendment 11, # ratified decision 8(d)): the helper's own `workstation` read is the one # implementation, and its second tab-separated field says which rung answered. @@ -1639,9 +1676,9 @@ if have_tmux; then window_name="$(tmux display-message -p '#W' 2>/dev/null || :)" win_harness="$(tmux display-message -p '#{session_name}:#{window_id}' 2>/dev/null || :)" fi -[ -n "$win" ] || win="${WORKBENCHES_CLAUDE_WINDOW_REF:-}" +[ -n "$win" ] || win="${OPENBENCHES_CLAUDE_WINDOW_REF:-${WORKBENCHES_CLAUDE_WINDOW_REF:-}}" window_ref_ok "$win" || win="" -[ -n "$wid" ] || wid="${WORKBENCHES_CLAUDE_WINDOW_ID:-}" +[ -n "$wid" ] || wid="${OPENBENCHES_CLAUDE_WINDOW_ID:-${WORKBENCHES_CLAUDE_WINDOW_ID:-}}" window_id_ok "$wid" || wid="" window_ref="${win}${wid:+ $wid}" @@ -1866,7 +1903,7 @@ if [ -z "$dir" ]; then *) dir=""; dir_read_failed="\`$LANES_EDIT lane-dir $lane\` failed (exit $dir_rc)" ;; esac if [ -z "$dir" ] && [ -z "$dir_read_failed" ]; then - dir="${WORKBENCHES_CLAUDE_LANE_DIR:-}" + dir="${OPENBENCHES_CLAUDE_LANE_DIR:-${WORKBENCHES_CLAUDE_LANE_DIR:-}}" if [ -z "$dir" ] && [ -n "${CLAUDE_CODE_SESSION_ID:-}" ] && [ -n "${CLAUDE_CONFIG_DIR:-}" ] && command -v jq >/dev/null 2>&1; then dir="$(jq -r --arg id "$CLAUDE_CODE_SESSION_ID" \ 'select((.sessionId // "") == $id) | .cwd // empty' \ diff --git a/skills/handoff/SKILL.md b/skills/handoff/SKILL.md index aba1d6d9..b8654d48 100644 --- a/skills/handoff/SKILL.md +++ b/skills/handoff/SKILL.md @@ -138,6 +138,53 @@ case "${why-}" in esac ``` +**THE LAUNCHER'S ENVIRONMENT IS NEW-FIRST WITH LEGACY FALLBACK, AND A DUAL CONFLICT IS A REFUSAL.** For +`WINDOW_REF`, `WINDOW_ID` and `LANE_DIR`, step 4 reads `OPENBENCHES_CLAUDE_*` first and then the corresponding +`WORKBENCHES_CLAUDE_*` name. A value supplied under only one name is used as before; equal old/new dual values +are accepted. If both names are set to different values, the skill refuses before lane selection, window +rename, handoff refresh or record writes rather than risking the wrong lane or checkout. Set both names to the +same value, or unset the incorrect one, and run `/handoff` again. A set-but-empty value still counts as set, +so an empty/non-empty pair conflicts too. These checks run now, ahead of step 1, because the directory and +window reads in step 4 are too late to protect step 2's writes. + +```sh +check_claude_env_dual() { # + local cce_suffix="$1" cce_new_set cce_legacy_set cce_new cce_legacy + case "$cce_suffix" in + WINDOW_REF) + cce_new_set="${OPENBENCHES_CLAUDE_WINDOW_REF+set}" + cce_legacy_set="${WORKBENCHES_CLAUDE_WINDOW_REF+set}" + cce_new="${OPENBENCHES_CLAUDE_WINDOW_REF-}" + cce_legacy="${WORKBENCHES_CLAUDE_WINDOW_REF-}" + ;; + WINDOW_ID) + cce_new_set="${OPENBENCHES_CLAUDE_WINDOW_ID+set}" + cce_legacy_set="${WORKBENCHES_CLAUDE_WINDOW_ID+set}" + cce_new="${OPENBENCHES_CLAUDE_WINDOW_ID-}" + cce_legacy="${WORKBENCHES_CLAUDE_WINDOW_ID-}" + ;; + LANE_DIR) + cce_new_set="${OPENBENCHES_CLAUDE_LANE_DIR+set}" + cce_legacy_set="${WORKBENCHES_CLAUDE_LANE_DIR+set}" + cce_new="${OPENBENCHES_CLAUDE_LANE_DIR-}" + cce_legacy="${WORKBENCHES_CLAUDE_LANE_DIR-}" + ;; + *) + printf 'REFUSED: unknown Claude environment suffix: %s\n' "$cce_suffix" >&2 + return 2 + ;; + esac + if [ "$cce_new_set" = set ] && [ "$cce_legacy_set" = set ] && [ "$cce_new" != "$cce_legacy" ]; then + printf 'REFUSED: OPENBENCHES_CLAUDE_%s and WORKBENCHES_CLAUDE_%s have conflicting values. Set both to the same value, or unset the incorrect one, then re-run. Nothing was renamed or written.\n' \ + "$cce_suffix" "$cce_suffix" >&2 + return 2 + fi +} +check_claude_env_dual WINDOW_REF || exit +check_claude_env_dual WINDOW_ID || exit +check_claude_env_dual LANE_DIR || exit +``` + ## 1. Usage, then the identity triple — derived, not asked ```bash @@ -542,7 +589,8 @@ below, and never a template placeholder (#80 item 7). # 1. the lane's own recorded `dir` (`lane-dir`), written by `lane-start` at # every start under this amendment — the lane's checkout as the lane's own # writer recorded it; -# 2. the launcher's `WORKBENCHES_CLAUDE_LANE_DIR`, exported only where the +# 2. the launcher's `OPENBENCHES_CLAUDE_LANE_DIR`, with legacy +# `WORKBENCHES_CLAUDE_LANE_DIR` fallback, exported only where the # directory order's rungs 1-3 answered; # 3. THE LIVE SESSION'S OWN RECORD — the harness writes `"cwd"` beside # `"tmux"` in `$CLAUDE_CONFIG_DIR/sessions/.json`, keyed by @@ -580,7 +628,7 @@ case "$dir_rc" in *) dir=""; dir_read_failed="\`$L lane-dir $lane\` failed (exit $dir_rc)" ;; esac if [[ -z "$dir" && -z "$dir_read_failed" ]]; then - dir="${WORKBENCHES_CLAUDE_LANE_DIR:-}" + dir="${OPENBENCHES_CLAUDE_LANE_DIR:-${WORKBENCHES_CLAUDE_LANE_DIR:-}}" if [[ -z "$dir" && -n "${CLAUDE_CODE_SESSION_ID:-}" && -n "${CLAUDE_CONFIG_DIR:-}" ]]; then dir="$(jq -r --arg id "$CLAUDE_CODE_SESSION_ID" \ 'select((.sessionId // "") == $id) | .cwd // empty' \ @@ -595,7 +643,8 @@ fi # SUPERSET, so a seventh divergence found after this is absorbed on the same # ground and needs no ruling of its own"*. On the normal path the launcher # started this session and threaded the window it captured across its re-exec -# (`WORKBENCHES_CLAUDE_WINDOW_REF`, `_WINDOW_ID`, clause (b) act 1); a `tmux` +# (`OPENBENCHES_CLAUDE_WINDOW_REF`, `_WINDOW_ID`, with the legacy +# `WORKBENCHES_CLAUDE_*` names as fallback, clause (b) act 1); a `tmux` # that is missing, a server that has gone, or a shim that answers nothing then # costs the record a `window` sub-field it already had in hand. Each is SHAPE # CHECKED before it is taken, by the same two tests the live reads get, because @@ -614,11 +663,11 @@ fi win="$(tmux display-message -p '#S:#I' 2>/dev/null)" [[ "$win" =~ ^.+:[0-9]+$ ]] || win="" case "$win" in *$'\n'*|*$'\r'*) win="" ;; esac -[[ -n "$win" ]] || win="${WORKBENCHES_CLAUDE_WINDOW_REF:-}" +[[ -n "$win" ]] || win="${OPENBENCHES_CLAUDE_WINDOW_REF:-${WORKBENCHES_CLAUDE_WINDOW_REF:-}}" [[ "$win" =~ ^.+:[0-9]+$ ]] || win="" case "$win" in *$'\n'*|*$'\r'*) win="" ;; esac wid="$(tmux display-message -p '#{window_id}' 2>/dev/null)" -[[ "$wid" =~ ^@[0-9]+$ ]] || wid="${WORKBENCHES_CLAUDE_WINDOW_ID:-}" +[[ "$wid" =~ ^@[0-9]+$ ]] || wid="${OPENBENCHES_CLAUDE_WINDOW_ID:-${WORKBENCHES_CLAUDE_WINDOW_ID:-}}" # AN `<@id>` IS `@` AND NOTHING ELSE (F-X13(a), A11 Addendum 4 ruling 10). # `== @*` accepted anything starting with an at-sign, and two things that are # not ids do: a tmux too old to know `#{window_id}` PRINTS THE FORMAT BACK, and diff --git a/tests/test_openbenches_handoff_compat.py b/tests/test_openbenches_handoff_compat.py new file mode 100644 index 00000000..879642b1 --- /dev/null +++ b/tests/test_openbenches_handoff_compat.py @@ -0,0 +1,254 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Old/new launcher environment compatibility for the two handoff surfaces. + +Every variable is exercised legacy-only, new-only, with equal dual values and +with conflicting dual values. The skill cases run its own fenced shell blocks; +the command cases run the real `lane-handoff` against an isolated register and +prove that a conflict is refused before any handoff or lane write. +""" +from __future__ import annotations + +import shlex +import shutil +import subprocess +from pathlib import Path + +import pytest + +from conftest import REPO, WINDOWS_SKIP +from test_handoff_skill_72_80 import ( + SHELLS, + Sandbox as SkillSandbox, + Verb, + _cut, + _one_block, + window_block, +) +from test_lane_start_claude_current import LANE, Sandbox as LaneSandbox, _write + + +pytestmark = [ + WINDOWS_SKIP, + pytest.mark.skipif(shutil.which("bash") is None, reason="handoff is bash"), + pytest.mark.skipif(shutil.which("git") is None, reason="the register is git"), +] + +SUFFIXES = ("WINDOW_REF", "WINDOW_ID", "LANE_DIR") +MODES = ("legacy-only", "new-only", "equal-dual", "conflicting-dual") + + +def _names(suffix): + return f"OPENBENCHES_CLAUDE_{suffix}", f"WORKBENCHES_CLAUDE_{suffix}" + + +def _clear_pair(env, suffix): + for name in _names(suffix): + env.pop(name, None) + + +def _set_pair(env, suffix, mode, new_value, legacy_value): + new_name, legacy_name = _names(suffix) + _clear_pair(env, suffix) + if mode == "legacy-only": + env[legacy_name] = legacy_value + elif mode == "new-only": + env[new_name] = new_value + elif mode == "equal-dual": + env[new_name] = new_value + env[legacy_name] = new_value + else: + env[new_name] = new_value + env[legacy_name] = legacy_value + + +def _assert_conflict(proc, suffix): + new_name, legacy_name = _names(suffix) + assert proc.returncode == 2, (proc.stdout, proc.stderr) + assert "AFTER" not in proc.stdout, proc.stdout + assert new_name in proc.stderr, proc.stderr + assert legacy_name in proc.stderr, proc.stderr + assert "conflicting values" in proc.stderr, proc.stderr + assert "unset the incorrect one" in proc.stderr, proc.stderr + assert "Nothing was renamed or written" in proc.stderr, proc.stderr + + +def _environment_block(): + return "\n".join(_one_block("check_claude_env_dual() {")) + "\n" + + +def _command_environment_block(): + source = (REPO / "lane-handoff").read_text() + start = source.index("check_claude_env_dual() {") + end = source.index("\n}\n", start) + 3 + return source[start:end] + ( + "check_claude_env_dual WINDOW_REF || exit\n" + "check_claude_env_dual WINDOW_ID || exit\n" + "check_claude_env_dual LANE_DIR || exit\n" + ) + + +def _directory_block(): + first = 'dir=""; dir_read_failed=' + return _cut(_one_block(first), first, "fi", exact_last=True) + + +@pytest.fixture +def skill_box(tmp_path: Path) -> SkillSandbox: + return SkillSandbox(tmp_path) + + +@pytest.mark.parametrize("shell", SHELLS) +@pytest.mark.parametrize("suffix", SUFFIXES) +@pytest.mark.parametrize("mode", MODES) +def test_openbenches_handoff_skill_environment_matrix(skill_box, shell, suffix, mode): + values = { + "WINDOW_REF": ("new:7", "legacy:4"), + "WINDOW_ID": ("@77", "@44"), + "LANE_DIR": ("/new/checkout", "/legacy/checkout"), + } + new_value, legacy_value = values[suffix] + env = {} + _set_pair(env, suffix, mode, new_value, legacy_value) + script = _environment_block() + if suffix == "LANE_DIR": + skill_box.stub({"lane-dir": Verb(8)}) + script = f"L={shlex.quote(str(skill_box.helper))}\nlane=repoOB-1\n" + script + script += _directory_block() + "printf 'AFTER [%s]\\n' \"$dir\"\n" + expected = new_value if mode in ("new-only", "equal-dual") else legacy_value + else: + if suffix == "WINDOW_REF": + env["WORKBENCHES_CLAUDE_WINDOW_ID"] = "@9" + selected = new_value if mode in ("new-only", "equal-dual") else legacy_value + expected = f"{selected} @9" + else: + env["WORKBENCHES_CLAUDE_WINDOW_REF"] = "sess:1" + selected = new_value if mode in ("new-only", "equal-dual") else legacy_value + expected = f"sess:1 {selected}" + script += "tmux() { printf '\\n'; }\n" + window_block() + script += "printf 'AFTER [%s]\\n' \"$win\"\n" + proc = skill_box.run(shell, script, env) + if mode == "conflicting-dual": + _assert_conflict(proc, suffix) + assert skill_box.logged() == [], skill_box.logged() + else: + assert proc.returncode == 0, (proc.stdout, proc.stderr) + assert proc.stdout == f"AFTER [{expected}]\n", (proc.stdout, proc.stderr) + assert proc.stderr == "", proc.stderr + + +@pytest.mark.parametrize("implementation", ("skill", "command")) +@pytest.mark.parametrize("shell", SHELLS) +@pytest.mark.parametrize("suffix", SUFFIXES) +def test_openbenches_handoff_environment_values_are_not_executed( + skill_box, shell, suffix, implementation, tmp_path +): + script = ( + _environment_block() + if implementation == "skill" + else _command_environment_block() + ) + script += "printf 'AFTER\\n'\n" + for index, payload in enumerate( + ("$(touch {marker})", "`touch {marker}`", '"; touch {marker}; #') + ): + marker = tmp_path / f"executed-{implementation}-{suffix}-{index}" + value = payload.format(marker=shlex.quote(str(marker))) + env = {} + _set_pair(env, suffix, "equal-dual", value, value) + proc = skill_box.run(shell, script, env) + assert proc.returncode == 0, (implementation, proc.stdout, proc.stderr) + assert proc.stdout == "AFTER\n", (implementation, proc.stdout, proc.stderr) + assert proc.stderr == "", (implementation, proc.stdout, proc.stderr) + assert not marker.exists(), (implementation, payload, proc.stdout, proc.stderr) + + +@pytest.fixture +def handoff_box(tmp_path: Path): + box = LaneSandbox(tmp_path) + for suffix in SUFFIXES: + _clear_pair(box.env, suffix) + box.env.update( + LANES_LANE_STATE_ROOT=str(tmp_path / "state"), + LANES_NO_FETCH="1", + TMUX_PANE="%1", + ) + box.resolver() + initial = box.start("--no-launch") + assert initial.returncode == 0, (initial.stdout, initial.stderr) + row = next( + line for line in (box.wip / "lanes" / "LANES.md").read_text().splitlines() + if line.startswith(f"| `{LANE}`") + ) + source = box.wip / row.split("|")[6].strip() + _write(source, "# openBenches compatibility\n\nPreserved task.\n---\nHistory.\n", 0o600) + box.git("-C", str(box.wip), "add", "--", str(source)) + box.git("-C", str(box.wip), "commit", "-qm", "seed handoff") + box.git("-C", str(box.wip), "push", "-q", "origin", "main") + shutil.copy2(REPO / "lane-handoff", box.bin / "lane-handoff") + real = box.bin / "lanes-edit-real.sh" + (box.bin / "lanes-edit.sh").rename(real) + _write( + box.bin / "lanes-edit.sh", + "#!/usr/bin/env bash\n" + "if [ \"$1\" = lane-dir ]; then exit 8; fi\n" + f"exec {shlex.quote(str(real))} \"$@\"\n", + ) + return box, source + + +def _handoff_environment(box, suffix, mode): + valid = { + "WINDOW_REF": "testsess:0", + "WINDOW_ID": "@1", + "LANE_DIR": str(box.lane_dir), + } + wrong = { + "WINDOW_REF": "othersess:9", + "WINDOW_ID": "@9", + "LANE_DIR": str(box.root / "wrong-checkout"), + } + env = {f"WORKBENCHES_CLAUDE_{name}": value for name, value in valid.items()} + _set_pair(env, suffix, mode, wrong[suffix] if mode == "conflicting-dual" else valid[suffix], valid[suffix]) + return env + + +def _run_handoff(box, extra): + env = dict(box.env) + for suffix in SUFFIXES: + _clear_pair(env, suffix) + env.update(extra) + env["LANE_HANDOFF_NO_TMUX"] = "1" + return subprocess.run( + [str(box.bin / "lane-handoff"), "--lane", LANE, "clear"], + cwd=box.lane_dir, + env=env, + text=True, + capture_output=True, + timeout=120, + check=False, + ) + + +@pytest.mark.parametrize("suffix", SUFFIXES) +@pytest.mark.parametrize("mode", MODES) +def test_openbenches_lane_handoff_environment_matrix(handoff_box, suffix, mode): + box, source = handoff_box + before = { + "source": source.read_bytes(), + "log": box.lane_log(), + "register": (box.wip / "lanes" / "LANES.md").read_bytes(), + "commits": box.commits(), + } + proc = _run_handoff(box, _handoff_environment(box, suffix, mode)) + if mode == "conflicting-dual": + _assert_conflict(proc, suffix) + assert source.read_bytes() == before["source"] + assert box.lane_log() == before["log"] + assert (box.wip / "lanes" / "LANES.md").read_bytes() == before["register"] + assert box.commits() == before["commits"] + return + assert proc.returncode == 0, (proc.stdout, proc.stderr) + paused = [line for line in box.lane_log().splitlines() if line.startswith("PAUSED")] + assert len(paused) == 1, box.lane_log() + assert "; window testsess:0 @1;" in paused[0], paused[0] + assert f"; dir {box.lane_dir};" in paused[0], paused[0]