fix(devin): make SessionEnd installation non-destructive

CHANGELOG.md

@@ -7,6 +7,8 @@ All notable changes to SkillOpt are documented here. This project adheres to
 ## [Unreleased]
 
 ### Added
+- A non-destructive Devin installer and SessionEnd activity marker, preserving
+  existing project hooks across repeated installation.
 - Per-night SkillOpt-Sleep `evidence.jsonl` chains for reconstructing harvest,
   mining, replay, reflection, and gate decisions, plus a live prompt-template
   registry with user overrides.

plugins/devin/README.md

@@ -18,7 +18,7 @@ source into the Claude Code-compatible JSONL the engine reads.
 | `mcp-config.example.json` | drop-in MCP server config |
 | `install.sh` | copies hooks + rules into a project's `.devin/` and prints the MCP registration command |
 | `devin-rules.snippet.md` | copied to `.devin/rules/skillopt-sleep.md` by `install.sh` |
-| `hooks/hooks.v1.json` | SessionEnd hook config — copied to `.devin/hooks.v1.json` by `install.sh` |
+| `hooks/hooks.v1.json` | SessionEnd hook config — installed/merged at `.devin/hooks.v1.json` by `install.sh` |
 | `hooks/on-session-end.sh` | best-effort activity marker script (called by the hook) |
 
 ## What it harvests
@@ -44,9 +44,11 @@ Requires Python ≥ 3.10. No third-party packages — the server is pure stdlib.
 
    This copies the SessionEnd hook and rules snippet into the project's
    `.devin/` directory and prints the MCP registration command. The hook is
-   on by default — it logs a cheap activity marker when each session ends so
-   the next nightly cycle knows there is fresh data to harvest. It is
-   non-blocking and spends no API budget. Re-run the script to update.
+   on by default — it logs a cheap activity marker for local inspection or
+   external automation when each session ends. The current engine harvests by
+   transcript timestamps and does not consume this marker directly. The hook
+   is non-blocking and spends no API budget. Re-run the script to update; the
+   installer preserves existing hooks and does not duplicate its own entry.
 
 2. **Register the MCP server.** Use `mcp-config.example.json` as a template, or
    run the command printed by `install.sh`:

plugins/devin/hooks/hooks.v1.json

@@ -5,7 +5,7 @@
       "hooks": [
         {
           "type": "command",
-          "command": "\"${DEVIN_PROJECT_DIR}/.devin/hooks/on-session-end.sh\"",
+          "command": "\"${DEVIN_PROJECT_DIR}/.devin/hooks/skillopt-sleep-on-session-end.sh\"",
           "timeout": 5
         }
       ]

plugins/devin/hooks/on-session-end.sh

@@ -1,19 +1,20 @@
 #!/usr/bin/env bash
 # SkillOpt-Sleep SessionEnd hook for Devin (best-effort, NON-BLOCKING).
 #
-# This does NOT run the optimizer. It only appends a tiny marker so the next
-# nightly cycle knows there is fresh activity to harvest, and (optionally)
-# nudges the user once that a sleep cycle is available. It must never fail the
-# session or spend API budget.
+# This does NOT run the optimizer. It only appends a tiny marker for local
+# inspection or external automation. The current sleep engine uses transcript
+# timestamps rather than this marker. The hook must never fail the session or
+# spend API budget.
 #
-# Install: copy this file and hooks.v1.json into your project's .devin/hooks/
-# directory. Devin CLI reads .devin/hooks.v1.json automatically.
+# Install this script as .devin/hooks/skillopt-sleep-on-session-end.sh and the
+# config at .devin/hooks.v1.json. Devin CLI reads it automatically.
 set -uo pipefail
 
+[ -n "${HOME:-}" ] || exit 0
 STATE_DIR="${HOME}/.skillopt-sleep"
 mkdir -p "$STATE_DIR" 2>/dev/null || exit 0
 
-# Record that a session just ended (cheap; used for "is there new data?").
+# Record that a session just ended (cheap local activity signal).
 printf '%s\t%s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "${DEVIN_PROJECT_DIR:-${PWD}}" \
   >> "$STATE_DIR/session-end.log" 2>/dev/null || true
 

plugins/devin/install.sh

@@ -18,24 +18,61 @@ mkdir -p "$DEVIN_DIR/hooks" "$DEVIN_DIR/rules"
 #    Merge into existing hooks.v1.json instead of overwriting, so we don't
 #    destroy other project hooks.
 HOOK_SCRIPT_SRC="$PLUGIN_DIR/hooks/on-session-end.sh"
-HOOK_SCRIPT_DST="$DEVIN_DIR/hooks/on-session-end.sh"
+HOOK_SCRIPT_DST="$DEVIN_DIR/hooks/skillopt-sleep-on-session-end.sh"
 cp "$HOOK_SCRIPT_SRC" "$HOOK_SCRIPT_DST"
 chmod +x "$HOOK_SCRIPT_DST"
 echo "[install] hook script       -> $HOOK_SCRIPT_DST"
 
 HOOK_CONFIG="$DEVIN_DIR/hooks.v1.json"
 if [ -f "$HOOK_CONFIG" ]; then
-  # Merge our SessionEnd hook into the existing config (jq deep-merge)
-  if command -v jq >/dev/null 2>&1; then
-    jq -s '.[0] * .[1]' "$HOOK_CONFIG" "$PLUGIN_DIR/hooks/hooks.v1.json" > "$HOOK_CONFIG.tmp"
-    mv "$HOOK_CONFIG.tmp" "$HOOK_CONFIG"
-    echo "[install] session-end hook  -> $HOOK_CONFIG (merged)"
-  else
-    echo "[install] WARNING: jq not found; cannot merge into existing $HOOK_CONFIG"
-    echo "[install] Merge this SessionEnd hook manually or install jq:"
-    echo "[install]   cat $PLUGIN_DIR/hooks/hooks.v1.json"
-    echo "[install] Skipping hook config to avoid overwriting existing hooks."
-  fi
+  # Python is already required by the plugin. Merge event arrays without
+  # replacing existing hooks, and skip exact duplicates on repeated installs.
+  python3 - "$HOOK_CONFIG" "$PLUGIN_DIR/hooks/hooks.v1.json" <<'PY'
+import json
+import os
+import stat
+import sys
+import tempfile
+
+destination, addition = sys.argv[1:]
+
+
+def load_object(path):
+    with open(path, encoding="utf-8") as handle:
+        value = json.load(handle)
+    if not isinstance(value, dict):
+        raise ValueError(f"hook config must be a JSON object: {path}")
+    return value
+
+
+base = load_object(destination)
+incoming = load_object(addition)
+for event, entries in incoming.items():
+    if not isinstance(entries, list):
+        raise ValueError(f"hook event {event!r} must be an array")
+    existing = base.setdefault(event, [])
+    if not isinstance(existing, list):
+        raise ValueError(f"existing hook event {event!r} must be an array")
+    for entry in entries:
+        if entry not in existing:
+            existing.append(entry)
+
+directory = os.path.dirname(os.path.abspath(destination))
+fd, temporary = tempfile.mkstemp(prefix=".hooks.v1.", suffix=".tmp", dir=directory)
+try:
+    with os.fdopen(fd, "w", encoding="utf-8") as handle:
+        json.dump(base, handle, ensure_ascii=False, indent=2)
+        handle.write("\n")
+    os.chmod(temporary, stat.S_IMODE(os.stat(destination).st_mode))
+    os.replace(temporary, destination)
+except Exception:
+    try:
+        os.unlink(temporary)
+    except OSError:
+        pass
+    raise
+PY
+  echo "[install] session-end hook  -> $HOOK_CONFIG (merged)"
 else
   cp "$PLUGIN_DIR/hooks/hooks.v1.json" "$HOOK_CONFIG"
   echo "[install] session-end hook  -> $HOOK_CONFIG"
@@ -46,13 +83,14 @@ cp "$PLUGIN_DIR/devin-rules.snippet.md" "$DEVIN_DIR/rules/skillopt-sleep.md"
 echo "[install] rules snippet     -> $DEVIN_DIR/rules/skillopt-sleep.md"
 
 # 3) Print the MCP server registration command
+printf -v MCP_SERVER_QUOTED '%q' "$PLUGIN_DIR/mcp_server.py"
 cat <<EOF
 
 [install] Register the MCP server (run once per machine):
 
   devin mcp add skillopt-sleep \\
     --env "SKILLOPT_DEVIN_CLAUDE_HOME=\$HOME/.skillopt-sleep-devin" \\
-    -- python3 $PLUGIN_DIR/mcp_server.py
+    -- python3 $MCP_SERVER_QUOTED
 
 Done. Try asking Devin:
   Run the sleep cycle for this project.

tests/test_devin_plugin.py

@@ -2,6 +2,10 @@
 import importlib
 import json
 import os
+import shlex
+import shutil
+import stat
+import subprocess
 import sys
 import tempfile
 import unittest
@@ -14,6 +18,7 @@
 import harvest_devin as hw   # noqa: E402
 
 FIXTURES = os.path.join(PLUGIN, "fixtures")
+INSTALLER = os.path.join(PLUGIN, "install.sh")
 
 
 def _read_jsonl(path):
@@ -79,6 +84,165 @@ def test_env_tilde_is_expanded(self):
             importlib.reload(mcp_server)
 
 
+class TestDevinInstaller(unittest.TestCase):
+    def _run_installer(self, project, home, installer=INSTALLER):
+        env = os.environ.copy()
+        env["HOME"] = home
+        return subprocess.run(
+            ["bash", installer, project],
+            check=True,
+            capture_output=True,
+            text=True,
+            env=env,
+        )
+
+    @staticmethod
+    def _skillopt_hook():
+        config_path = os.path.join(PLUGIN, "hooks", "hooks.v1.json")
+        with open(config_path, encoding="utf-8") as f:
+            return json.load(f)["SessionEnd"][0]
+
+    def test_new_install_and_hook_marker(self):
+        with tempfile.TemporaryDirectory() as d:
+            project = os.path.join(d, "project with spaces")
+            home = os.path.join(d, "home")
+            os.makedirs(project)
+            os.makedirs(home)
+
+            self._run_installer(project, home)
+
+            config_path = os.path.join(project, ".devin", "hooks.v1.json")
+            with open(config_path, encoding="utf-8") as f:
+                config = json.load(f)
+            self.assertEqual(config["SessionEnd"], [self._skillopt_hook()])
+
+            hook_path = os.path.join(
+                project, ".devin", "hooks", "skillopt-sleep-on-session-end.sh"
+            )
+            self.assertTrue(os.stat(hook_path).st_mode & stat.S_IXUSR)
+            env = os.environ.copy()
+            env.update(HOME=home, DEVIN_PROJECT_DIR=project)
+            subprocess.run([hook_path], check=True, env=env)
+            marker = os.path.join(home, ".skillopt-sleep", "session-end.log")
+            with open(marker, encoding="utf-8") as f:
+                lines = f.readlines()
+            self.assertEqual(len(lines), 1)
+            self.assertTrue(lines[0].endswith(f"\t{project}\n"))
+
+    def test_hook_is_non_blocking_without_home(self):
+        env = os.environ.copy()
+        env.pop("HOME", None)
+        result = subprocess.run(
+            [os.path.join(PLUGIN, "hooks", "on-session-end.sh")],
+            capture_output=True,
+            text=True,
+            env=env,
+        )
+        self.assertEqual(result.returncode, 0)
+        self.assertEqual(result.stderr, "")
+
+    def test_existing_config_without_session_end_is_extended(self):
+        unrelated = [
+            {"matcher": "", "hooks": [{"type": "command", "command": "./pre.sh"}]}
+        ]
+        with tempfile.TemporaryDirectory() as d:
+            project = os.path.join(d, "project")
+            home = os.path.join(d, "home")
+            devin_dir = os.path.join(project, ".devin")
+            os.makedirs(devin_dir)
+            os.makedirs(home)
+            config_path = os.path.join(devin_dir, "hooks.v1.json")
+            with open(config_path, "w", encoding="utf-8") as f:
+                json.dump({"PreToolUse": unrelated}, f)
+
+            self._run_installer(project, home)
+
+            with open(config_path, encoding="utf-8") as f:
+                config = json.load(f)
+            self.assertEqual(config["PreToolUse"], unrelated)
+            self.assertEqual(config["SessionEnd"], [self._skillopt_hook()])
+
+    def test_existing_hooks_are_preserved_and_reinstall_is_idempotent(self):
+        existing_session_end = {
+            "matcher": "existing",
+            "hooks": [{"type": "command", "command": "./existing.sh"}],
+        }
+        unrelated = [
+            {"matcher": "", "hooks": [{"type": "command", "command": "./pre.sh"}]}
+        ]
+        with tempfile.TemporaryDirectory() as d:
+            project = os.path.join(d, "project")
+            home = os.path.join(d, "home")
+            devin_dir = os.path.join(project, ".devin")
+            os.makedirs(devin_dir)
+            os.makedirs(home)
+            hooks_dir = os.path.join(devin_dir, "hooks")
+            os.makedirs(hooks_dir)
+            legacy_hook = os.path.join(hooks_dir, "on-session-end.sh")
+            with open(legacy_hook, "w", encoding="utf-8") as f:
+                f.write("#!/bin/sh\n# existing project hook\n")
+            config_path = os.path.join(devin_dir, "hooks.v1.json")
+            with open(config_path, "w", encoding="utf-8") as f:
+                json.dump(
+                    {"PreToolUse": unrelated, "SessionEnd": [existing_session_end]}, f
+                )
+
+            self._run_installer(project, home)
+            self._run_installer(project, home)
+
+            with open(config_path, encoding="utf-8") as f:
+                config = json.load(f)
+            self.assertEqual(config["PreToolUse"], unrelated)
+            self.assertIn(existing_session_end, config["SessionEnd"])
+            self.assertEqual(config["SessionEnd"].count(self._skillopt_hook()), 1)
+            with open(legacy_hook, encoding="utf-8") as f:
+                self.assertEqual(f.read(), "#!/bin/sh\n# existing project hook\n")
+
+    def test_malformed_existing_config_fails_without_overwrite(self):
+        with tempfile.TemporaryDirectory() as d:
+            project = os.path.join(d, "project")
+            home = os.path.join(d, "home")
+            devin_dir = os.path.join(project, ".devin")
+            os.makedirs(devin_dir)
+            os.makedirs(home)
+            config_path = os.path.join(devin_dir, "hooks.v1.json")
+            original = "{not-json\n"
+            with open(config_path, "w", encoding="utf-8") as f:
+                f.write(original)
+
+            with self.assertRaises(subprocess.CalledProcessError):
+                self._run_installer(project, home)
+            with open(config_path, encoding="utf-8") as f:
+                self.assertEqual(f.read(), original)
+
+    def test_registration_path_is_shell_quoted(self):
+        with tempfile.TemporaryDirectory() as d:
+            plugin_copy = os.path.join(
+                d, "repo with spaces $dollar `tick` 'quote'", "plugins", "devin"
+            )
+            shutil.copytree(PLUGIN, plugin_copy)
+            project = os.path.join(d, "project")
+            home = os.path.join(d, "home")
+            os.makedirs(project)
+            os.makedirs(home)
+
+            result = self._run_installer(
+                project,
+                home,
+                installer=os.path.join(plugin_copy, "install.sh"),
+            )
+
+            command_line = next(
+                line.strip()
+                for line in result.stdout.splitlines()
+                if line.strip().startswith("-- python3 ")
+            )
+            self.assertEqual(
+                shlex.split(command_line),
+                ["--", "python3", os.path.join(plugin_copy, "mcp_server.py")],
+            )
+
+
 class TestDevinHarvest(unittest.TestCase):
     def test_atif_fixture_yields_gradeable_task(self):
         with tempfile.TemporaryDirectory() as out: