diff --git a/README.md b/README.md index fe9eb98..0da983f 100644 --- a/README.md +++ b/README.md @@ -247,6 +247,23 @@ collaboration. | `/inter-session:inter-session disconnect` | Stop the monitor. | | `/inter-session:inter-session auto-start [on\|off\|status]` | Toggle auto-start. `on` = start at every session; `off` = lazy (default). Apply with `/reload-plugins`. | +## Session labels + +Alongside its `name` (the ASCII handle used for addressing), a session can +carry an optional **label** — a short Unicode display string (up to 60 +characters, e.g. `Payments 🐛 refund bug`) shown in the `list` table. Labels +are display-only; you always address a session by its `name`. + +Set one when the monitor starts with the client's `--label` flag. A label set +this way is **remembered per project** — persisted in the data dir, keyed by +the git repo root (falling back to the working directory outside a repo) — so +it is reused automatically on the next restart without re-passing the flag: + +- `--label "…"` — set and persist for this project. +- `--label ""` — clear the persisted label. +- `INTER_SESSION_LABEL` — a one-off runtime override; used but **not** + persisted. + ## Plugin configuration The WebSocket port and idle-shutdown timeout are configurable via diff --git a/README.zh.md b/README.zh.md index 5720987..a2e9379 100644 --- a/README.zh.md +++ b/README.zh.md @@ -189,6 +189,20 @@ blue → "tracked in PATCH_LOG.md (24 entries). all classes hardened. | `/inter-session:inter-session disconnect` | 停止 monitor。 | | `/inter-session:inter-session auto-start [on\|off\|status]` | 切换 auto-start。`on` = 每次会话启动时连接;`off` = 懒启动(默认)。改动后用 `/reload-plugins` 应用。 | +## 会话标签(label) + +除了用于寻址的 `name`(ASCII 句柄)之外,会话还可以带一个可选的 **label** —— +一个简短的 Unicode 显示字符串(最多 60 个字符,例如 `Payments 🐛 refund bug`), +显示在 `list` 表格中。label 仅用于显示;寻址始终使用 `name`。 + +在 monitor 启动时通过客户端的 `--label` 参数设置。以这种方式设置的 label 会 +**按项目记住** —— 持久化到数据目录,以 git 仓库根目录为键(不在仓库中时回退到 +当前工作目录)—— 因此下次重启会自动复用,无需再次传入该参数: + +- `--label "…"` —— 为当前项目设置并持久化。 +- `--label ""` —— 清除已持久化的 label。 +- `INTER_SESSION_LABEL` —— 一次性的运行时覆盖;会被使用但**不会**持久化。 + ## 插件配置 通过 `/plugin config` 可配置 WebSocket 端口与空闲关闭超时: diff --git a/skills/inter-session/SKILL.md b/skills/inter-session/SKILL.md index 33a03e9..1c586ac 100644 --- a/skills/inter-session/SKILL.md +++ b/skills/inter-session/SKILL.md @@ -161,6 +161,13 @@ Works the same whether the skill is installed as part of the plugin so leave them off. Use plain `python3` — `client.py` re-execs under the project venv automatically once `install-deps` has created it. + **Optional `--label`**: to give the session a human-friendly display + string (shown in `list`; addressing still uses `name`), add + `--label ""` to the command. A label set this way **persists per + project** — it's remembered (keyed by the git repo root) and reused on + the next connect without re-passing it, so only pass `--label` when the + user asks to set or change it. `--label ""` clears the persisted label. + Each stdout line is a peer message — apply the Reaction policy above. 3. **If the spawn returns diff --git a/skills/inter-session/bin/client.py b/skills/inter-session/bin/client.py index b5e7bcd..4c67af3 100644 --- a/skills/inter-session/bin/client.py +++ b/skills/inter-session/bin/client.py @@ -37,7 +37,7 @@ if str(_REPO_ROOT) not in sys.path: sys.path.insert(0, str(_REPO_ROOT)) -from bin import shared, spawn +from bin import shared, spawn, profile log = logging.getLogger("inter-session.client") @@ -68,33 +68,10 @@ def _format_truncation_pointer(msg_id: str, full_len: int) -> str: def _write_session_state(ppid: int, state: dict) -> None: - """Atomic write: create a sibling tempfile, fchmod 0600, fsync, then - os.replace. This prevents helpers from observing a partially-written - state file (the previous direct `write_text` was non-atomic). - """ - import tempfile + """Atomically write the listener state file (0600), so helpers never + observe a partially-written file.""" shared.secure_dir(shared.clients_dir()) - path = shared.client_session_path(ppid) - parent = path.parent - fd, tmp_path = tempfile.mkstemp( - prefix=path.name + ".", suffix=".tmp", dir=str(parent), - ) - try: - os.fchmod(fd, 0o600) - os.write(fd, (json.dumps(state) + "\n").encode("utf-8")) - os.fsync(fd) - os.close(fd) - os.replace(tmp_path, str(path)) - except OSError: - try: - os.close(fd) - except OSError: - pass - try: - os.unlink(tmp_path) - except OSError: - pass - raise + shared.atomic_write_text(shared.client_session_path(ppid), json.dumps(state) + "\n") def _delete_session_state(ppid: int) -> None: @@ -355,6 +332,29 @@ async def _ping_loop(self, ws) -> None: return +def _resolve_label(cli_label, env_label, cwd=None) -> str: + """Resolve (and persist) the session's display label. Precedence: + + 1. `--label` given (cli_label is not None): validate, persist it for this + project ('' clears the persisted label), and use it. + 2. else `$INTER_SESSION_LABEL` (env_label truthy): validate and use as a + one-off runtime override — NOT persisted. + 3. else: load the persisted per-project label (or ''). + + Raises ValueError(bad_label) if an explicitly-supplied label is invalid, so + the caller can surface it and exit. + """ + if cli_label is not None: + if not shared.validate_label(cli_label): + raise ValueError(cli_label) + return profile.resolve_label(cli_label, cwd) + if env_label: + if not shared.validate_label(env_label): + raise ValueError(env_label) + return env_label + return profile.resolve_label(None, cwd) + + def _env_int(*keys, default: int) -> int: for k in keys: v = os.environ.get(k) @@ -389,7 +389,9 @@ def main() -> int: "CLAUDE_PLUGIN_OPTION_PORT", "INTER_SESSION_PORT", default=shared.DEFAULT_PORT, )) parser.add_argument("--name", default=os.environ.get("INTER_SESSION_NAME", "")) - parser.add_argument("--label", default=os.environ.get("INTER_SESSION_LABEL", "")) + # Default None (not "") so we can tell "flag absent" (→ load the persisted + # per-project label) apart from an explicit "--label ''" (→ clear it). + parser.add_argument("--label", default=None) parser.add_argument("--idle-shutdown-minutes", type=float, default=_env_float( "CLAUDE_PLUGIN_OPTION_IDLE_SHUTDOWN_MINUTES", "INTER_SESSION_IDLE_MINUTES", default=10, @@ -404,8 +406,11 @@ def main() -> int: if args.name and not shared.validate_name(args.name): _print_line(f"[inter-session] invalid name {args.name!r}") return 1 - if not shared.validate_label(args.label): - _print_line(f"[inter-session] invalid label {args.label!r}") + + try: + final_label = _resolve_label(args.label, os.environ.get("INTER_SESSION_LABEL")) + except ValueError as e: + _print_line(f"[inter-session] invalid label {e.args[0]!r}") return 1 # Plugin auto-start path: monitors.json doesn't pass --name (so the user @@ -423,7 +428,7 @@ def main() -> int: ) client = Client( - host=args.host, port=args.port, name=final_name, label=args.label, + host=args.host, port=args.port, name=final_name, label=final_label, idle_shutdown_minutes=args.idle_shutdown_minutes, verbose=args.verbose, ) diff --git a/skills/inter-session/bin/profile.py b/skills/inter-session/bin/profile.py new file mode 100644 index 0000000..86f518d --- /dev/null +++ b/skills/inter-session/bin/profile.py @@ -0,0 +1,99 @@ +"""Per-project identity profile: persists a session's display label so it is +reused across restarts without re-passing --label. + +Scope (this feature): label only. The profile is a small JSON file in the data +dir, keyed by the *project root* — the nearest ancestor directory containing a +`.git` entry, else the working directory. Keying by repo root (rather than raw +cwd) means `cd`-ing into a subdirectory of the same repo resolves the same +profile. The stored dict carries a `path`/`updated_at` and room for future +fields, so adding e.g. `team` later needs no migration. + +Values are re-validated on load, so a hand-edited or older-format file can +never surface a label the live validation path would reject. +""" + +from __future__ import annotations + +import hashlib +import json +import os +from datetime import datetime, timezone +from pathlib import Path +from typing import Optional + +from bin import shared + + +def profiles_dir() -> Path: + return shared.data_dir() / "profiles" + + +def project_root(cwd: Optional[str] = None) -> str: + """Stable per-project key: the nearest ancestor directory containing a + `.git` entry (a dir for a normal repo, a file for a worktree/submodule), + else the starting directory. Symlink-resolved so equivalent paths collapse + to one profile.""" + start = Path(cwd) if cwd else Path.cwd() + try: + start = start.resolve() + except OSError: + start = start.absolute() + for d in (start, *start.parents): + if (d / ".git").exists(): + return str(d) + return str(start) + + +def profile_path(cwd: Optional[str] = None) -> Path: + """Data-dir path of the profile for the current project. The filename is a + hash we generate from the project root, so no part of it is caller-supplied + (no path-traversal surface).""" + digest = hashlib.sha256(project_root(cwd).encode("utf-8")).hexdigest()[:32] + return profiles_dir() / f"{digest}.json" + + +def _read(cwd: Optional[str] = None) -> dict: + try: + data = json.loads(profile_path(cwd).read_text()) + except (OSError, json.JSONDecodeError): + return {} + return data if isinstance(data, dict) else {} + + +def load_label(cwd: Optional[str] = None) -> str: + """The persisted label for this project, or '' if none/invalid/corrupt.""" + label = _read(cwd).get("label", "") + if isinstance(label, str) and shared.validate_label(label): + return label + return "" + + +def save_label(label: str, cwd: Optional[str] = None) -> None: + """Persist `label` for this project; an empty string clears it. Other keys + in the profile are preserved (forward-compat). An invalid non-empty label + is a no-op (the live path rejects it separately).""" + if label and not shared.validate_label(label): + return + shared.secure_dir(profiles_dir()) + data = _read(cwd) + data["path"] = project_root(cwd) + if label: + data["label"] = label + else: + data.pop("label", None) + data["updated_at"] = datetime.now(tz=timezone.utc).isoformat() + shared.atomic_write_text(profile_path(cwd), json.dumps(data)) + + +def resolve_label(explicit: Optional[str], cwd: Optional[str] = None) -> str: + """Resolve the label to connect with. + + `explicit` is None when the caller passed no explicit label (→ load and + return the persisted per-project label, or ''), or a string (possibly '') + when the caller passed one explicitly (→ persist it, '' clearing, and + return it). The caller is responsible for validating a non-empty explicit + label first; save_label is a defensive no-op on an invalid one.""" + if explicit is None: + return load_label(cwd) + save_label(explicit, cwd) + return explicit diff --git a/skills/inter-session/bin/shared.py b/skills/inter-session/bin/shared.py index 4670048..0b914c0 100644 --- a/skills/inter-session/bin/shared.py +++ b/skills/inter-session/bin/shared.py @@ -232,6 +232,33 @@ def rotate_log_if_needed(path: Path, max_bytes: int, backups: int) -> None: pass +def atomic_write_text(path: Path, text: str, mode: int = 0o600) -> None: + """Atomically write `text` to `path` with `mode` perms: write a sibling + tempfile (fchmod + fsync), then os.replace so readers never observe a + partial file. The parent directory must already exist (callers create it + with the right perms via secure_dir).""" + import tempfile + path = Path(path) + fd, tmp = tempfile.mkstemp(prefix=path.name + ".", suffix=".tmp", + dir=str(path.parent)) + try: + os.fchmod(fd, mode) + os.write(fd, text.encode("utf-8")) + os.fsync(fd) + os.close(fd) + os.replace(tmp, str(path)) + except OSError: + try: + os.close(fd) + except OSError: + pass + try: + os.unlink(tmp) + except OSError: + pass + raise + + def safe_pid_alive(pid: int) -> bool: """True if a process with `pid` is alive (or we can signal 0 to it).""" if pid <= 0: diff --git a/tests/test_client.py b/tests/test_client.py index 0465a40..bcf7f81 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -66,6 +66,57 @@ def _read_until_nonempty(proc, timeout=5.0): return "" +class TestResolveLabel: + """client._resolve_label: --label vs $INTER_SESSION_LABEL vs persisted.""" + + def test_flag_persists_and_is_used(self, tmp_data_dir, tmp_path): + cwd = str(tmp_path) + assert client_mod._resolve_label("Payments 🐛", None, cwd) == "Payments 🐛" + assert client_mod.profile.load_label(cwd) == "Payments 🐛" # persisted + + def test_flag_empty_clears(self, tmp_data_dir, tmp_path): + cwd = str(tmp_path) + client_mod._resolve_label("old", None, cwd) + assert client_mod._resolve_label("", None, cwd) == "" + assert client_mod.profile.load_label(cwd) == "" + + def test_env_used_but_not_persisted(self, tmp_data_dir, tmp_path): + cwd = str(tmp_path) + assert client_mod._resolve_label(None, "env-label", cwd) == "env-label" + assert client_mod.profile.load_label(cwd) == "" # NOT persisted + + def test_flag_takes_precedence_over_env(self, tmp_data_dir, tmp_path): + cwd = str(tmp_path) + assert client_mod._resolve_label("flag", "env", cwd) == "flag" + + def test_absent_loads_persisted(self, tmp_data_dir, tmp_path): + cwd = str(tmp_path) + client_mod.profile.save_label("stored", cwd) + assert client_mod._resolve_label(None, None, cwd) == "stored" + + def test_empty_env_falls_back_to_persisted(self, tmp_data_dir, tmp_path): + cwd = str(tmp_path) + client_mod.profile.save_label("stored", cwd) + assert client_mod._resolve_label(None, "", cwd) == "stored" + + def test_absent_with_nothing_is_empty(self, tmp_data_dir, tmp_path): + assert client_mod._resolve_label(None, None, str(tmp_path)) == "" + + def test_invalid_flag_raises(self, tmp_data_dir, tmp_path): + with pytest.raises(ValueError): + client_mod._resolve_label("a\nb", None, str(tmp_path)) + # and nothing was persisted + assert client_mod.profile.load_label(str(tmp_path)) == "" + + def test_invalid_env_raises(self, tmp_data_dir, tmp_path): + with pytest.raises(ValueError): + client_mod._resolve_label(None, "a\nb", str(tmp_path)) + + def test_over_max_length_flag_raises(self, tmp_data_dir, tmp_path): + with pytest.raises(ValueError): + client_mod._resolve_label("a" * (shared.LABEL_MAX_CP + 1), None, str(tmp_path)) + + class TestFormatMsg: def test_basic_msg(self): msg = {"op": "msg", "msg_id": "ab12", "from": "x", "from_name": "alpha", diff --git a/tests/test_profile.py b/tests/test_profile.py new file mode 100644 index 0000000..c82add3 --- /dev/null +++ b/tests/test_profile.py @@ -0,0 +1,159 @@ +"""Per-project label persistence (bin/profile.py).""" + +from __future__ import annotations + +import json + +import pytest + +from bin import profile, shared + + +@pytest.fixture +def tmp_data_dir(tmp_path, monkeypatch): + d = tmp_path / "inter-session" + monkeypatch.setenv("INTER_SESSION_DATA_DIR", str(d)) + return d + + +class TestProjectRoot: + def test_git_ancestor_is_root(self, tmp_path): + (tmp_path / ".git").mkdir() + sub = tmp_path / "a" / "b" + sub.mkdir(parents=True) + assert profile.project_root(str(sub)) == str(tmp_path.resolve()) + + def test_git_file_worktree_is_root(self, tmp_path): + # Worktrees/submodules use a `.git` *file*, not a directory. + (tmp_path / ".git").write_text("gitdir: /elsewhere") + sub = tmp_path / "x" + sub.mkdir() + assert profile.project_root(str(sub)) == str(tmp_path.resolve()) + + def test_no_repo_falls_back_to_cwd(self, tmp_path): + d = tmp_path / "plain" + d.mkdir() + assert profile.project_root(str(d)) == str(d.resolve()) + + def test_same_repo_subdirs_share_profile(self, tmp_path): + (tmp_path / ".git").mkdir() + a = tmp_path / "a" + b = tmp_path / "a" / "deep" / "nested" + b.mkdir(parents=True) + a.mkdir(exist_ok=True) + assert profile.profile_path(str(a)) == profile.profile_path(str(b)) + + +class TestLoadSave: + def test_load_missing_is_empty(self, tmp_data_dir, tmp_path): + assert profile.load_label(str(tmp_path)) == "" + + def test_save_then_load(self, tmp_data_dir, tmp_path): + profile.save_label("Payments 🐛 v2", str(tmp_path)) + assert profile.load_label(str(tmp_path)) == "Payments 🐛 v2" + + def test_save_empty_clears(self, tmp_data_dir, tmp_path): + profile.save_label("keep", str(tmp_path)) + profile.save_label("", str(tmp_path)) + assert profile.load_label(str(tmp_path)) == "" + + def test_profile_file_mode_0600(self, tmp_data_dir, tmp_path): + import os + import stat + profile.save_label("x", str(tmp_path)) + p = profile.profile_path(str(tmp_path)) + assert stat.S_IMODE(os.stat(p).st_mode) == 0o600 + + def test_invalid_stored_label_ignored(self, tmp_data_dir, tmp_path): + # A hand-edited/corrupt file with a label the live path would reject + # must not surface it. + shared.secure_dir(profile.profiles_dir()) + p = profile.profile_path(str(tmp_path)) + p.write_text(json.dumps({"label": "bad\nlabel"})) # newline is invalid + assert profile.load_label(str(tmp_path)) == "" + + def test_corrupt_json_ignored(self, tmp_data_dir, tmp_path): + shared.secure_dir(profile.profiles_dir()) + profile.profile_path(str(tmp_path)).write_text("{not json") + assert profile.load_label(str(tmp_path)) == "" + + def test_invalid_label_not_saved(self, tmp_data_dir, tmp_path): + profile.save_label("a\nb", str(tmp_path)) + assert not profile.profile_path(str(tmp_path)).exists() + + def test_save_preserves_other_keys(self, tmp_data_dir, tmp_path): + shared.secure_dir(profile.profiles_dir()) + p = profile.profile_path(str(tmp_path)) + p.write_text(json.dumps({"team": "payments", "label": "old"})) + profile.save_label("new", str(tmp_path)) + data = json.loads(p.read_text()) + assert data["label"] == "new" + assert data["team"] == "payments" # forward-compat field untouched + assert "updated_at" in data and "path" in data + + +class TestLabelLengthBoundary: + def test_save_and_load_at_max_length(self, tmp_data_dir, tmp_path): + label = "a" * shared.LABEL_MAX_CP # exactly 60 — valid + profile.save_label(label, str(tmp_path)) + assert profile.load_label(str(tmp_path)) == label + + def test_save_over_max_length_is_noop(self, tmp_data_dir, tmp_path): + # A label longer than the maximum must not be persisted at all. + over = "a" * (shared.LABEL_MAX_CP + 1) # 61 — invalid + profile.save_label(over, str(tmp_path)) + assert not profile.profile_path(str(tmp_path)).exists() + assert profile.load_label(str(tmp_path)) == "" + + def test_save_over_max_does_not_overwrite_existing(self, tmp_data_dir, tmp_path): + profile.save_label("keep", str(tmp_path)) + profile.save_label("a" * (shared.LABEL_MAX_CP + 1), str(tmp_path)) + assert profile.load_label(str(tmp_path)) == "keep" # prior value survives + + def test_load_ignores_over_max_length_on_disk(self, tmp_data_dir, tmp_path): + # A hand-edited file with an over-length label is ignored on load. + shared.secure_dir(profile.profiles_dir()) + profile.profile_path(str(tmp_path)).write_text( + json.dumps({"label": "a" * (shared.LABEL_MAX_CP + 1)}) + ) + assert profile.load_label(str(tmp_path)) == "" + + +class TestMalformedProfile: + def test_load_non_dict_json_ignored(self, tmp_data_dir, tmp_path): + shared.secure_dir(profile.profiles_dir()) + profile.profile_path(str(tmp_path)).write_text(json.dumps(["not", "a", "dict"])) + assert profile.load_label(str(tmp_path)) == "" + + def test_load_non_string_label_ignored(self, tmp_data_dir, tmp_path): + shared.secure_dir(profile.profiles_dir()) + profile.profile_path(str(tmp_path)).write_text(json.dumps({"label": 123})) + assert profile.load_label(str(tmp_path)) == "" + + def test_load_missing_label_key(self, tmp_data_dir, tmp_path): + shared.secure_dir(profile.profiles_dir()) + profile.profile_path(str(tmp_path)).write_text(json.dumps({"team": "x"})) + assert profile.load_label(str(tmp_path)) == "" + + +class TestResolveLabel: + def test_none_loads_persisted(self, tmp_data_dir, tmp_path): + profile.save_label("stored", str(tmp_path)) + assert profile.resolve_label(None, str(tmp_path)) == "stored" + + def test_none_with_nothing_persisted_is_empty(self, tmp_data_dir, tmp_path): + assert profile.resolve_label(None, str(tmp_path)) == "" + + def test_explicit_persists_and_returns(self, tmp_data_dir, tmp_path): + assert profile.resolve_label("set-me", str(tmp_path)) == "set-me" + assert profile.load_label(str(tmp_path)) == "set-me" # persisted + + def test_explicit_empty_clears_and_returns_empty(self, tmp_data_dir, tmp_path): + profile.save_label("old", str(tmp_path)) + assert profile.resolve_label("", str(tmp_path)) == "" + assert profile.load_label(str(tmp_path)) == "" # cleared + + def test_explicit_overrides_persisted(self, tmp_data_dir, tmp_path): + profile.save_label("old", str(tmp_path)) + assert profile.resolve_label("new", str(tmp_path)) == "new" + assert profile.load_label(str(tmp_path)) == "new" diff --git a/tests/test_shared.py b/tests/test_shared.py index d3a3fc2..1d92d0c 100644 --- a/tests/test_shared.py +++ b/tests/test_shared.py @@ -183,6 +183,31 @@ def test_rejects_symlink(self, tmp_path): shared.ensure_token(link) +class TestAtomicWriteText: + def test_writes_content(self, tmp_path): + p = tmp_path / "f.json" + shared.atomic_write_text(p, '{"a": 1}') + assert p.read_text() == '{"a": 1}' + + def test_mode_0600_by_default(self, tmp_path): + p = tmp_path / "f" + shared.atomic_write_text(p, "x") + assert stat.S_IMODE(os.stat(p).st_mode) == 0o600 + + def test_overwrites_atomically(self, tmp_path): + p = tmp_path / "f" + shared.atomic_write_text(p, "first") + shared.atomic_write_text(p, "second") + assert p.read_text() == "second" + # No tempfiles left behind. + assert [x.name for x in tmp_path.iterdir()] == ["f"] + + def test_custom_mode(self, tmp_path): + p = tmp_path / "f" + shared.atomic_write_text(p, "x", mode=0o644) + assert stat.S_IMODE(os.stat(p).st_mode) == 0o644 + + class TestSecureDir: def test_creates_with_0700(self, tmp_path): d = tmp_path / "data"