From adde53850ce990913e406409db56edd558478d88 Mon Sep 17 00:00:00 2001 From: "Christopher Haugen (from Dev Box)" Date: Tue, 4 Aug 2026 15:13:52 +0200 Subject: [PATCH 1/5] feat(sleep): harvest GitHub Copilot CLI sessions SkillOpt-Sleep could only read VS Code Copilot Chat transcripts, so users whose work happens in the Copilot CLI had almost nothing to mine. On one machine the VS Code source yielded 3 sessions and 1 task -- too thin for the gate to distinguish a real improvement from a formatting trick. The Copilot CLI keeps a global SQLite index at ~/.copilot/session-store.db with per-session cwd/branch and per-turn user/assistant text. On the same machine it yields 1326 harvestable sessions and 6587 user turns, 475 of them carrying pos/neg feedback signals usable as labels. Reading that index avoids parsing the multi-gigabyte per-session events.jsonl logs for data the CLI already indexes. The store is written by live sessions, so reads go through a read-only connection and fall back to a private snapshot when the live WAL cannot be opened read-only; a harvest must never block or corrupt an in-flight session. Engine self-calls are filtered. A Copilot-backed sleep run writes to this same store, so an unfiltered harvest mines the engine's own rollout and reflect prompts -- confirmed empirically before the filter was added. The existing _is_headless_replay/_is_agent_session guards are applied, and the replay markers now also cover the analyst prompts and the '## Skill' rollout header. Enable with --source copilot_cli; override the store path with --copilot-cli-session-store. --- skillopt_sleep/__main__.py | 11 +- skillopt_sleep/config.py | 11 +- skillopt_sleep/harvest.py | 4 + skillopt_sleep/harvest_copilot_cli.py | 180 +++++++++++++++++++++++++ skillopt_sleep/harvest_sources.py | 9 ++ tests/test_harvest_copilot_cli.py | 186 ++++++++++++++++++++++++++ 6 files changed, 398 insertions(+), 3 deletions(-) create mode 100644 skillopt_sleep/harvest_copilot_cli.py create mode 100644 tests/test_harvest_copilot_cli.py diff --git a/skillopt_sleep/__main__.py b/skillopt_sleep/__main__.py index c87cde0d..416922c3 100644 --- a/skillopt_sleep/__main__.py +++ b/skillopt_sleep/__main__.py @@ -14,8 +14,9 @@ --target-skill-path PATH explicit live SKILL.md to stage/adopt --tasks-file PATH reviewed TaskRecord JSON file to replay instead of harvesting --backend mock|claude|codex|copilot|cursor|pi|handoff|azure_openai - --source claude|codex|copilot|cursor|pi|auto + --source claude|codex|copilot|copilot_cli|cursor|pi|auto --vscode-workspace-storage PATH + --copilot-cli-session-store PATH --model NAME --lookback-hours N --auto-adopt @@ -83,10 +84,12 @@ def _add_common(p: argparse.ArgumentParser) -> None: p.add_argument("--cursor-home", default="", help="override ~/.cursor for Cursor session harvest") p.add_argument("--pi-home", default="", help="override ~/.pi for Pi session harvest") p.add_argument("--source", default="", - choices=["", "claude", "codex", "copilot", "cursor", "pi", "auto"], + choices=["", "claude", "codex", "copilot", "copilot_cli", "cursor", "pi", "auto"], help="session transcript source") p.add_argument("--vscode-workspace-storage", default="", help="override VS Code User/workspaceStorage root for copilot source") + p.add_argument("--copilot-cli-session-store", default="", + help="override ~/.copilot/session-store.db for copilot_cli source") p.add_argument("--lookback-hours", type=int, default=None, help="harvest window in hours; 0 = scan full history") p.add_argument("--edit-budget", type=int, default=0) @@ -137,6 +140,10 @@ def _cfg_from_args(args, task_meta: Dict[str, Any] | None = None) -> Any: overrides["vscode_workspace_storage"] = os.path.abspath( os.path.expanduser(args.vscode_workspace_storage) ) + if getattr(args, "copilot_cli_session_store", ""): + overrides["copilot_cli_session_store"] = os.path.abspath( + os.path.expanduser(args.copilot_cli_session_store) + ) lh = getattr(args, "lookback_hours", None) if lh is not None: # --lookback-hours was explicitly passed (0 = full history) overrides["lookback_hours"] = lh diff --git a/skillopt_sleep/config.py b/skillopt_sleep/config.py index 982a6b75..8c429a09 100644 --- a/skillopt_sleep/config.py +++ b/skillopt_sleep/config.py @@ -30,7 +30,9 @@ "pi_home": PI_HOME, "cursor_home": CURSOR_HOME, "vscode_workspace_storage": "", # "" => auto-detect platform defaults - # Explicit sources also include copilot, cursor, and pi. ``auto`` keeps + "copilot_cli_session_store": "", # "" => ~/.copilot/session-store.db + # Explicit sources also include copilot, copilot_cli, cursor, and pi. + # ``auto`` keeps # the established Codex-then-Claude precedence for backward compatibility. "transcript_source": "claude", "projects": "invoked", # "invoked" | "all" | [list of abs paths] @@ -138,6 +140,13 @@ def cursor_projects_dir(self) -> str: cursor_home = os.path.abspath(os.path.expanduser(str(self.data["cursor_home"]))) return os.path.join(cursor_home, "projects") + @property + def copilot_cli_session_store(self) -> str: + value = self.data.get("copilot_cli_session_store", "") or "" + if not value: + return "" + return os.path.abspath(os.path.expanduser(str(value))) + @property def vscode_workspace_storage(self) -> str: value = self.data.get("vscode_workspace_storage", "") or "" diff --git a/skillopt_sleep/harvest.py b/skillopt_sleep/harvest.py index d0c102ca..7a9e2721 100644 --- a/skillopt_sleep/harvest.py +++ b/skillopt_sleep/harvest.py @@ -161,8 +161,12 @@ def _is_meta_prompt(text: str) -> bool: "You are a strict grader", "Score the response 0.0-1.0", "You are SkillOpt-Sleep", + "You are an expert success-pattern analyst", + "You are an expert failure-pattern analyst", "## TASK\n", "## SKILL\n", + # Engine rollouts render the skill under this heading before the task. + "## Skill\n", ) diff --git a/skillopt_sleep/harvest_copilot_cli.py b/skillopt_sleep/harvest_copilot_cli.py new file mode 100644 index 00000000..9f7cc926 --- /dev/null +++ b/skillopt_sleep/harvest_copilot_cli.py @@ -0,0 +1,180 @@ +"""SkillOpt-Sleep GitHub Copilot CLI session harvesting. + +The Copilot CLI keeps a global SQLite index at ``~/.copilot/session-store.db`` +that already carries everything a :class:`SessionDigest` needs: per-session +``cwd``/``branch`` and per-turn user/assistant text. Reading it is far cheaper +than replaying the multi-gigabyte per-session ``events.jsonl`` logs, and it is +the same data the CLI itself exposes. + +The store is written by live CLI sessions, so all reads go through a read-only +connection and fall back to a private snapshot when the live WAL cannot be +opened read-only. A harvest must never block or corrupt an in-flight session. +""" + +from __future__ import annotations + +import os +import shutil +import sqlite3 +import tempfile +from typing import Any, List, Optional + +from skillopt_sleep.harvest import ( + _detect_feedback, + _is_agent_session, + _is_headless_replay, + _is_meta_prompt, + _project_matches, +) +from skillopt_sleep.types import SessionDigest + +# Bound per-session text so one pathological session cannot dominate a night's +# harvest. Mining only needs intent, not a full transcript. +_MAX_PROMPTS_PER_SESSION = 40 +_MAX_FINALS_PER_SESSION = 40 +_MAX_TEXT_CHARS = 4000 + + +def default_session_store() -> str: + """Return the default Copilot CLI session-store path.""" + return os.path.join(os.path.expanduser("~"), ".copilot", "session-store.db") + + +def _clip(text: Any) -> str: + if not isinstance(text, str): + return "" + text = text.strip() + return text[:_MAX_TEXT_CHARS] + + +def _connect(store_path: str) -> tuple[sqlite3.Connection, Optional[str]]: + """Open ``store_path`` read-only, snapshotting if the live WAL blocks it. + + Returns the connection and the temp directory to clean up, if any. + """ + try: + con = sqlite3.connect(f"file:{store_path}?mode=ro", uri=True) + con.execute("SELECT 1 FROM sessions LIMIT 1").fetchone() + return con, None + except sqlite3.Error: + pass + + tmpdir = tempfile.mkdtemp(prefix="skillopt-sleep-copilot-cli-") + snapshot = os.path.join(tmpdir, "session-store.db") + shutil.copyfile(store_path, snapshot) + for suffix in ("-wal", "-shm"): + sidecar = store_path + suffix + if os.path.exists(sidecar): + shutil.copyfile(sidecar, snapshot + suffix) + return sqlite3.connect(f"file:{snapshot}?mode=ro", uri=True), tmpdir + + +def harvest_copilot_cli( + session_store: str = "", + *, + scope: Any = "all", + invoked_project: str = "", + since_iso: Optional[str] = None, + limit: int = 0, +) -> List[SessionDigest]: + """Read the Copilot CLI session store and return digests matching scope/time.""" + store_path = session_store or default_session_store() + if not os.path.isfile(store_path): + return [] + + con, tmpdir = _connect(store_path) + try: + con.row_factory = sqlite3.Row + params: list[Any] = [] + where = "" + if since_iso: + # Timestamps mix "YYYY-MM-DD HH:MM:SS" and ISO-8601 text, which only + # compare safely at day granularity. + where = "WHERE substr(created_at, 1, 10) >= substr(?, 1, 10)" + params.append(since_iso) + rows = con.execute( + "SELECT id, cwd, repository, branch, created_at, updated_at " + f"FROM sessions {where} ORDER BY updated_at DESC", + params, + ).fetchall() + + digests: List[SessionDigest] = [] + for row in rows: + project = row["cwd"] or row["repository"] or "" + if not _project_matches(project, scope, invoked_project): + continue + + turns = con.execute( + "SELECT user_message, assistant_response FROM turns WHERE session_id = ? ORDER BY turn_index", + (row["id"],), + ).fetchall() + + prompts: List[str] = [] + finals: List[str] = [] + feedback: List[str] = [] + n_user = 0 + n_asst = 0 + for turn in turns: + user_text = _clip(turn["user_message"]) + if user_text: + n_user += 1 + feedback.extend(_detect_feedback(user_text)) + if not _is_meta_prompt(user_text) and len(prompts) < _MAX_PROMPTS_PER_SESSION: + prompts.append(user_text) + asst_text = _clip(turn["assistant_response"]) + if asst_text: + n_asst += 1 + if len(finals) < _MAX_FINALS_PER_SESSION: + finals.append(asst_text) + + if not prompts: + continue + + files = [ + r["file_path"] + for r in con.execute( + "SELECT DISTINCT file_path FROM session_files WHERE session_id = ?", + (row["id"],), + ) + if r["file_path"] + ] + tools = sorted( + { + r["tool_name"] + for r in con.execute( + "SELECT DISTINCT tool_name FROM session_files WHERE session_id = ?", + (row["id"],), + ) + if r["tool_name"] + } + ) + + digests.append( + SessionDigest( + session_id=str(row["id"]), + project=project, + git_branch=row["branch"] or "", + started_at=row["created_at"] or "", + ended_at=row["updated_at"] or "", + user_prompts=prompts, + assistant_finals=finals, + tools_used=tools, + files_touched=files, + feedback_signals=sorted(set(feedback)), + n_user_turns=n_user, + n_assistant_turns=n_asst, + raw_path=f"{store_path}#{row['id']}", + ) + ) + # SkillOpt's own Copilot backend calls land in this same store, so + # an unfiltered harvest would train the engine on its own output. + if _is_headless_replay(digests[-1]) or _is_agent_session(digests[-1]): + digests.pop() + continue + if limit and len(digests) >= limit: + break + return digests + finally: + con.close() + if tmpdir: + shutil.rmtree(tmpdir, ignore_errors=True) diff --git a/skillopt_sleep/harvest_sources.py b/skillopt_sleep/harvest_sources.py index 2cb4e384..ec526c2f 100644 --- a/skillopt_sleep/harvest_sources.py +++ b/skillopt_sleep/harvest_sources.py @@ -6,6 +6,7 @@ from skillopt_sleep.harvest import harvest from skillopt_sleep.harvest_codex import harvest_codex from skillopt_sleep.harvest_copilot import harvest_copilot +from skillopt_sleep.harvest_copilot_cli import harvest_copilot_cli from skillopt_sleep.harvest_cursor import harvest_cursor from skillopt_sleep.harvest_pi import harvest_pi from skillopt_sleep.types import SessionDigest @@ -32,6 +33,14 @@ def harvest_for_config(cfg, *, since_iso: Optional[str] = None, limit: int = 0) since_iso=since_iso, limit=limit, ) + if source == "copilot_cli": + return harvest_copilot_cli( + cfg.copilot_cli_session_store, + scope=scope, + invoked_project=invoked_project, + since_iso=since_iso, + limit=limit, + ) if source == "cursor": return harvest_cursor( cfg.cursor_projects_dir, diff --git a/tests/test_harvest_copilot_cli.py b/tests/test_harvest_copilot_cli.py new file mode 100644 index 00000000..e344942c --- /dev/null +++ b/tests/test_harvest_copilot_cli.py @@ -0,0 +1,186 @@ +from __future__ import annotations + +import os +import sqlite3 + +import pytest + +from skillopt_sleep.harvest_copilot_cli import default_session_store, harvest_copilot_cli + +_SCHEMA = """ +CREATE TABLE sessions ( + id TEXT PRIMARY KEY, + cwd TEXT, + repository TEXT, + branch TEXT, + summary TEXT, + created_at TEXT, + updated_at TEXT, + host_type TEXT +); +CREATE TABLE turns ( + id INTEGER PRIMARY KEY, + session_id TEXT, + turn_index INTEGER, + user_message TEXT, + assistant_response TEXT, + timestamp TEXT +); +CREATE TABLE session_files ( + session_id TEXT, + file_path TEXT, + tool_name TEXT +); +""" + + +def _store(tmp_path, sessions, turns, files=()): + path = os.path.join(str(tmp_path), "session-store.db") + con = sqlite3.connect(path) + con.executescript(_SCHEMA) + con.executemany( + "INSERT INTO sessions (id, cwd, repository, branch, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?)", + sessions, + ) + con.executemany( + "INSERT INTO turns (session_id, turn_index, user_message, assistant_response, timestamp) " + "VALUES (?, ?, ?, ?, ?)", + turns, + ) + if files: + con.executemany( + "INSERT INTO session_files (session_id, file_path, tool_name) VALUES (?, ?, ?)", + files, + ) + con.commit() + con.close() + return path + + +def test_missing_store_returns_empty(tmp_path) -> None: + assert harvest_copilot_cli(os.path.join(str(tmp_path), "nope.db")) == [] + + +def test_default_store_points_at_copilot_home() -> None: + assert default_session_store().endswith(os.path.join(".copilot", "session-store.db")) + + +def test_maps_session_and_turn_fields(tmp_path) -> None: + path = _store( + tmp_path, + [("s1", r"C:\proj", "repo", "main", "2026-01-01 10:00:00", "2026-01-01 10:30:00")], + [ + ("s1", 0, "Find the Nille repo", "Found it at C:/e", "2026-01-01 10:00:00"), + ("s1", 1, "that is still broken", "Fixed now", "2026-01-01 10:20:00"), + ], + [("s1", r"C:\proj\a.py", "edit")], + ) + digests = harvest_copilot_cli(path, scope="all") + + assert len(digests) == 1 + d = digests[0] + assert d.session_id == "s1" + assert d.project == r"C:\proj" + assert d.git_branch == "main" + assert d.n_user_turns == 2 + assert d.n_assistant_turns == 2 + assert "Find the Nille repo" in d.user_prompts + assert d.files_touched == [r"C:\proj\a.py"] + assert d.tools_used == ["edit"] + # "still broken" is a negative-feedback phrase and must survive as a label. + assert any(s.startswith("neg:") for s in d.feedback_signals) + assert d.raw_path.endswith("#s1") + + +def test_engine_self_calls_are_filtered(tmp_path) -> None: + # SkillOpt's own Copilot backend writes to this same store; harvesting them + # would train the engine on its own output. + path = _store( + tmp_path, + [ + ("real", r"C:\proj", "", "main", "2026-01-01 10:00:00", "2026-01-01 10:30:00"), + ("engine", r"C:\proj", "", "main", "2026-01-01 11:00:00", "2026-01-01 11:00:02"), + ], + [ + ("real", 0, "Review and rebase the PR", "Done", "2026-01-01 10:00:00"), + ( + "engine", + 0, + "You are an expert question answering agent.\n\n## Skill\n# QA Skill\n", + "Oslo", + "2026-01-01 11:00:00", + ), + ], + ) + ids = [d.session_id for d in harvest_copilot_cli(path, scope="all")] + assert ids == ["real"] + + +def test_scope_invoked_filters_by_project(tmp_path) -> None: + path = _store( + tmp_path, + [ + ("a", r"C:\projA", "", "", "2026-01-01 10:00:00", "2026-01-01 10:30:00"), + ("b", r"C:\projB", "", "", "2026-01-01 11:00:00", "2026-01-01 11:30:00"), + ], + [ + ("a", 0, "task in A", "ok", "2026-01-01 10:00:00"), + ("b", 0, "task in B", "ok", "2026-01-01 11:00:00"), + ], + ) + ids = [d.session_id for d in harvest_copilot_cli(path, scope="invoked", invoked_project=r"C:\projA")] + assert ids == ["a"] + + +def test_since_iso_compares_at_day_granularity(tmp_path) -> None: + # Timestamps mix "YYYY-MM-DD HH:MM:SS" and ISO-8601, so only the date part + # is safe to compare. + path = _store( + tmp_path, + [ + ("old", r"C:\p", "", "", "2026-01-01 10:00:00", "2026-01-01 10:20:00"), + ("new", r"C:\p", "", "", "2026-06-01T10:00:00.000Z", "2026-06-01T10:20:00.000Z"), + ], + [ + ("old", 0, "old task", "ok", "2026-01-01 10:00:00"), + ("new", 0, "new task", "ok", "2026-06-01T10:00:00.000Z"), + ], + ) + ids = [d.session_id for d in harvest_copilot_cli(path, scope="all", since_iso="2026-05-01")] + assert ids == ["new"] + + +def test_limit_caps_results(tmp_path) -> None: + sessions = [(f"s{i}", r"C:\p", "", "", f"2026-01-0{i} 10:00:00", f"2026-01-0{i} 10:00:00") for i in range(1, 6)] + turns = [(f"s{i}", 0, f"task {i}", "ok", f"2026-01-0{i} 10:00:00") for i in range(1, 6)] + path = _store(tmp_path, sessions, turns) + assert len(harvest_copilot_cli(path, scope="all", limit=2)) == 2 + + +def test_sessions_without_usable_prompts_are_skipped(tmp_path) -> None: + path = _store( + tmp_path, + [("empty", r"C:\p", "", "", "2026-01-01 10:00:00", "2026-01-01 10:00:00")], + [("empty", 0, "", "orphan answer", "2026-01-01 10:00:00")], + ) + assert harvest_copilot_cli(path, scope="all") == [] + + +def test_short_programmatic_sessions_are_filtered(tmp_path) -> None: + # A sub-3-second single-turn session with a short prompt is an engine call, + # not interactive work. + path = _store( + tmp_path, + [("quick", r"C:\p", "", "", "2026-01-01T10:00:00.000Z", "2026-01-01T10:00:01.000Z")], + [("quick", 0, "ping", "pong", "2026-01-01T10:00:00.000Z")], + ) + assert harvest_copilot_cli(path, scope="all") == [] + + +@pytest.mark.parametrize("source", ["copilot_cli"]) +def test_source_is_registered(source: str) -> None: + from skillopt_sleep import harvest_sources + + text = open(harvest_sources.__file__, encoding="utf-8").read() + assert f'source == "{source}"' in text + assert "harvest_copilot_cli" in text From fb930391246d0e89da96d0e84014b953db164de9 Mon Sep 17 00:00:00 2001 From: Christopher Haugen <1143916+lufen@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:44:17 +0200 Subject: [PATCH 2/5] fix(sleep): address review on Copilot CLI harvester - Build the read-only SQLite URI with pathname2url so Windows paths (drive letters, backslashes) and URI-special characters open correctly. - Fail closed (return []) when _connect() raises, honoring the 'never block a live run' guarantee for locked/unreadable/permission-denied stores. - Skip sessions without a stable cwd instead of falling back to repository or '', which is not abspath-able and collides on project+intent hashing. - Normalize 'YYYY-MM-DD HH:MM:SS' timestamps to ISO 'T' form so the shared sub-3s replay heuristic filters short programmatic sessions. - Close the file handle in the source-registration test via a with-block. - Add regressions for missing-cwd skip, space-timestamp filtering, and fail-closed connect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skillopt_sleep/harvest_copilot_cli.py | 49 +++++++++++++++++++++++---- tests/test_harvest_copilot_cli.py | 42 +++++++++++++++++++++-- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/skillopt_sleep/harvest_copilot_cli.py b/skillopt_sleep/harvest_copilot_cli.py index 9f7cc926..9359daef 100644 --- a/skillopt_sleep/harvest_copilot_cli.py +++ b/skillopt_sleep/harvest_copilot_cli.py @@ -18,6 +18,7 @@ import sqlite3 import tempfile from typing import Any, List, Optional +from urllib.request import pathname2url from skillopt_sleep.harvest import ( _detect_feedback, @@ -47,13 +48,39 @@ def _clip(text: Any) -> str: return text[:_MAX_TEXT_CHARS] +def _ro_uri(path: str) -> str: + """Build a read-only ``file:`` URI from a filesystem path. + + String-interpolating a raw path breaks on Windows (backslashes, ``C:`` + drive letters) and on any path containing URI-special characters, and can + silently prevent the read-only open. ``pathname2url`` produces a correctly + escaped, absolute URI on every platform. + """ + return "file:" + pathname2url(os.path.abspath(path)) + "?mode=ro" + + +def _norm_ts(value: Any) -> str: + """Normalize ``YYYY-MM-DD HH:MM:SS`` to ISO ``T`` form. + + The Copilot CLI store uses a space separator, but the shared + :func:`_is_headless_replay` duration heuristic strptimes a ``T``-separated + timestamp; without this, short programmatic sessions slip past the filter. + """ + if not isinstance(value, str): + return "" + v = value.strip() + if len(v) >= 19 and v[10] == " ": + v = v[:10] + "T" + v[11:] + return v + + def _connect(store_path: str) -> tuple[sqlite3.Connection, Optional[str]]: """Open ``store_path`` read-only, snapshotting if the live WAL blocks it. Returns the connection and the temp directory to clean up, if any. """ try: - con = sqlite3.connect(f"file:{store_path}?mode=ro", uri=True) + con = sqlite3.connect(_ro_uri(store_path), uri=True) con.execute("SELECT 1 FROM sessions LIMIT 1").fetchone() return con, None except sqlite3.Error: @@ -66,7 +93,7 @@ def _connect(store_path: str) -> tuple[sqlite3.Connection, Optional[str]]: sidecar = store_path + suffix if os.path.exists(sidecar): shutil.copyfile(sidecar, snapshot + suffix) - return sqlite3.connect(f"file:{snapshot}?mode=ro", uri=True), tmpdir + return sqlite3.connect(_ro_uri(snapshot), uri=True), tmpdir def harvest_copilot_cli( @@ -82,7 +109,12 @@ def harvest_copilot_cli( if not os.path.isfile(store_path): return [] - con, tmpdir = _connect(store_path) + try: + con, tmpdir = _connect(store_path) + except (sqlite3.Error, OSError): + # A harvest must never block or abort a run: a locked, unreadable, or + # permission-denied store simply yields no digests. + return [] try: con.row_factory = sqlite3.Row params: list[Any] = [] @@ -100,7 +132,12 @@ def harvest_copilot_cli( digests: List[SessionDigest] = [] for row in rows: - project = row["cwd"] or row["repository"] or "" + # A session without a stable cwd cannot be scoped (_project_matches + # needs an abspath) and would collide with others when mine.py + # hashes project+intent, so skip it -- as harvest_copilot() does. + project = row["cwd"] + if not project: + continue if not _project_matches(project, scope, invoked_project): continue @@ -154,8 +191,8 @@ def harvest_copilot_cli( session_id=str(row["id"]), project=project, git_branch=row["branch"] or "", - started_at=row["created_at"] or "", - ended_at=row["updated_at"] or "", + started_at=_norm_ts(row["created_at"]), + ended_at=_norm_ts(row["updated_at"]), user_prompts=prompts, assistant_finals=finals, tools_used=tools, diff --git a/tests/test_harvest_copilot_cli.py b/tests/test_harvest_copilot_cli.py index e344942c..da15d878 100644 --- a/tests/test_harvest_copilot_cli.py +++ b/tests/test_harvest_copilot_cli.py @@ -151,7 +151,7 @@ def test_since_iso_compares_at_day_granularity(tmp_path) -> None: def test_limit_caps_results(tmp_path) -> None: - sessions = [(f"s{i}", r"C:\p", "", "", f"2026-01-0{i} 10:00:00", f"2026-01-0{i} 10:00:00") for i in range(1, 6)] + sessions = [(f"s{i}", r"C:\p", "", "", f"2026-01-0{i} 10:00:00", f"2026-01-0{i} 10:30:00") for i in range(1, 6)] turns = [(f"s{i}", 0, f"task {i}", "ok", f"2026-01-0{i} 10:00:00") for i in range(1, 6)] path = _store(tmp_path, sessions, turns) assert len(harvest_copilot_cli(path, scope="all", limit=2)) == 2 @@ -181,6 +181,44 @@ def test_short_programmatic_sessions_are_filtered(tmp_path) -> None: def test_source_is_registered(source: str) -> None: from skillopt_sleep import harvest_sources - text = open(harvest_sources.__file__, encoding="utf-8").read() + with open(harvest_sources.__file__, encoding="utf-8") as fh: + text = fh.read() assert f'source == "{source}"' in text assert "harvest_copilot_cli" in text + + +def test_session_without_cwd_is_skipped(tmp_path) -> None: + # No stable cwd -> not scopable and would collide on project+intent hashing. + path = _store( + tmp_path, + [("nocwd", None, "owner/repo", "main", "2026-01-01 10:00:00", "2026-01-01 10:30:00")], + [("nocwd", 0, "do a real task here", "ok", "2026-01-01 10:00:00")], + ) + assert harvest_copilot_cli(path, scope="all") == [] + + +def test_short_session_with_space_timestamps_is_filtered(tmp_path) -> None: + # The store uses a space separator; normalization must let the sub-3-second + # replay heuristic fire just as it does for 'T'-separated ISO timestamps. + path = _store( + tmp_path, + [("quick", r"C:\p", "", "", "2026-01-01 10:00:00", "2026-01-01 10:00:01")], + [("quick", 0, "ping", "pong", "2026-01-01 10:00:00")], + ) + assert harvest_copilot_cli(path, scope="all") == [] + + +def test_connect_failure_fails_closed(tmp_path, monkeypatch) -> None: + # A locked/unreadable store must yield nothing rather than abort the run. + path = _store( + tmp_path, + [("s1", r"C:\p", "", "", "2026-01-01 10:00:00", "2026-01-01 10:30:00")], + [("s1", 0, "a real task", "ok", "2026-01-01 10:00:00")], + ) + + def _boom(_store_path): + raise OSError("permission denied") + + monkeypatch.setattr("skillopt_sleep.harvest_copilot_cli._connect", _boom) + assert harvest_copilot_cli(path, scope="all") == [] + From c052faa73e9363dc79b94da4b094ae365ef0e2cc Mon Sep 17 00:00:00 2001 From: Christopher Haugen <1143916+lufen@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:00:09 +0200 Subject: [PATCH 3/5] fix(sleep): harden Copilot CLI harvester per review follow-ups - Open both the live and snapshot read-only connections with timeout=0 so lock contention fails fast to the snapshot path instead of blocking a live run. - Filter since_iso on updated_at (session end) rather than created_at, so a long-lived session that ended after the cutoff is not dropped. - Keep only the last few assistant answers (rolling window of 5), matching the other harvesters; mining only reads assistant_finals[-1]. - Replace the brittle source-text assertion with a behavior test that harvest_for_config actually dispatches copilot_cli to the harvester. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skillopt_sleep/harvest_copilot_cli.py | 22 +++++++++----- tests/test_harvest_copilot_cli.py | 44 +++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/skillopt_sleep/harvest_copilot_cli.py b/skillopt_sleep/harvest_copilot_cli.py index 9359daef..ead28146 100644 --- a/skillopt_sleep/harvest_copilot_cli.py +++ b/skillopt_sleep/harvest_copilot_cli.py @@ -30,9 +30,11 @@ from skillopt_sleep.types import SessionDigest # Bound per-session text so one pathological session cannot dominate a night's -# harvest. Mining only needs intent, not a full transcript. +# harvest. Mining only needs intent, not a full transcript. Only the final few +# assistant answers matter downstream (mining reads assistant_finals[-1]), so +# we keep just the last few, consistent with the other harvesters. _MAX_PROMPTS_PER_SESSION = 40 -_MAX_FINALS_PER_SESSION = 40 +_MAX_FINALS_PER_SESSION = 5 _MAX_TEXT_CHARS = 4000 @@ -80,7 +82,7 @@ def _connect(store_path: str) -> tuple[sqlite3.Connection, Optional[str]]: Returns the connection and the temp directory to clean up, if any. """ try: - con = sqlite3.connect(_ro_uri(store_path), uri=True) + con = sqlite3.connect(_ro_uri(store_path), uri=True, timeout=0) con.execute("SELECT 1 FROM sessions LIMIT 1").fetchone() return con, None except sqlite3.Error: @@ -93,7 +95,7 @@ def _connect(store_path: str) -> tuple[sqlite3.Connection, Optional[str]]: sidecar = store_path + suffix if os.path.exists(sidecar): shutil.copyfile(sidecar, snapshot + suffix) - return sqlite3.connect(_ro_uri(snapshot), uri=True), tmpdir + return sqlite3.connect(_ro_uri(snapshot), uri=True, timeout=0), tmpdir def harvest_copilot_cli( @@ -120,9 +122,12 @@ def harvest_copilot_cli( params: list[Any] = [] where = "" if since_iso: + # since_iso is a cutoff on when a session *ended*, matching the + # other harvesters; filter on updated_at so a long-lived session + # that started earlier but ended after the cutoff is still kept. # Timestamps mix "YYYY-MM-DD HH:MM:SS" and ISO-8601 text, which only # compare safely at day granularity. - where = "WHERE substr(created_at, 1, 10) >= substr(?, 1, 10)" + where = "WHERE substr(updated_at, 1, 10) >= substr(?, 1, 10)" params.append(since_iso) rows = con.execute( "SELECT id, cwd, repository, branch, created_at, updated_at " @@ -161,8 +166,11 @@ def harvest_copilot_cli( asst_text = _clip(turn["assistant_response"]) if asst_text: n_asst += 1 - if len(finals) < _MAX_FINALS_PER_SESSION: - finals.append(asst_text) + # Keep only the last few answers (rolling window) so a long + # session does not balloon the digest. + finals.append(asst_text) + if len(finals) > _MAX_FINALS_PER_SESSION: + finals.pop(0) if not prompts: continue diff --git a/tests/test_harvest_copilot_cli.py b/tests/test_harvest_copilot_cli.py index da15d878..fd9a50dd 100644 --- a/tests/test_harvest_copilot_cli.py +++ b/tests/test_harvest_copilot_cli.py @@ -177,14 +177,46 @@ def test_short_programmatic_sessions_are_filtered(tmp_path) -> None: assert harvest_copilot_cli(path, scope="all") == [] -@pytest.mark.parametrize("source", ["copilot_cli"]) -def test_source_is_registered(source: str) -> None: +def test_copilot_cli_source_dispatches_via_harvest_for_config(monkeypatch) -> None: + # Behavior over text: harvest_for_config must actually route + # transcript_source="copilot_cli" to the harvester with the config's args. from skillopt_sleep import harvest_sources + from skillopt_sleep.config import SleepConfig + + seen: dict = {} + + def _fake(store, *, scope, invoked_project, since_iso, limit): + seen.update( + store=store, scope=scope, invoked_project=invoked_project, + since_iso=since_iso, limit=limit, + ) + return ["digest"] + + monkeypatch.setattr(harvest_sources, "harvest_copilot_cli", _fake) + cfg = SleepConfig() + cfg.data["transcript_source"] = "copilot_cli" + cfg.data["copilot_cli_session_store"] = r"C:\x\store.db" + cfg.data["projects"] = "all" + + out = harvest_sources.harvest_for_config(cfg, since_iso="2026-01-01", limit=5) + assert out == ["digest"] + assert seen["store"] == r"C:\x\store.db" + assert seen["scope"] == "all" + assert seen["since_iso"] == "2026-01-01" + assert seen["limit"] == 5 + + +def test_since_iso_filters_on_session_end_not_start(tmp_path) -> None: + # A long-lived session that started before the cutoff but ended after it + # must be kept (filter is on updated_at, not created_at). + path = _store( + tmp_path, + [("long", r"C:\p", "", "", "2026-04-30 23:00:00", "2026-05-02 01:00:00")], + [("long", 0, "a task spanning the cutoff", "ok", "2026-04-30 23:00:00")], + ) + ids = [d.session_id for d in harvest_copilot_cli(path, scope="all", since_iso="2026-05-01")] + assert ids == ["long"] - with open(harvest_sources.__file__, encoding="utf-8") as fh: - text = fh.read() - assert f'source == "{source}"' in text - assert "harvest_copilot_cli" in text def test_session_without_cwd_is_skipped(tmp_path) -> None: From 331d6d700bf62503ae13b0d86e5c6cfeb9c3d73f Mon Sep 17 00:00:00 2001 From: Christopher Haugen <1143916+lufen@users.noreply.github.com> Date: Wed, 5 Aug 2026 08:02:56 +0200 Subject: [PATCH 4/5] fix(sleep): harden harvester further per re-review - Remove an unused pytest import (F401) left after the test rewrite. - ORDER BY file_path so files_touched is deterministic (SessionDigest is hashed/persisted downstream). - _connect: close the half-open connection on the read-only failure path, validate the snapshot schema, and clean up the temp dir if snapshotting fails (re-raising so the caller fails closed). - Catch sqlite3.Error around the main query loop so schema drift/corruption mid-read yields [] instead of aborting the run (regression added). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- skillopt_sleep/harvest_copilot_cli.py | 36 +++++++++++++++++++-------- tests/test_harvest_copilot_cli.py | 18 ++++++++++++-- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/skillopt_sleep/harvest_copilot_cli.py b/skillopt_sleep/harvest_copilot_cli.py index ead28146..eeb88f2a 100644 --- a/skillopt_sleep/harvest_copilot_cli.py +++ b/skillopt_sleep/harvest_copilot_cli.py @@ -79,23 +79,35 @@ def _norm_ts(value: Any) -> str: def _connect(store_path: str) -> tuple[sqlite3.Connection, Optional[str]]: """Open ``store_path`` read-only, snapshotting if the live WAL blocks it. - Returns the connection and the temp directory to clean up, if any. + Returns the connection and the temp directory to clean up, if any. On any + failure the temp directory is removed and the error is re-raised so the + caller can fail closed. """ + con = None try: con = sqlite3.connect(_ro_uri(store_path), uri=True, timeout=0) con.execute("SELECT 1 FROM sessions LIMIT 1").fetchone() return con, None except sqlite3.Error: - pass + # Close the half-open connection before falling back to a snapshot. + if con is not None: + con.close() tmpdir = tempfile.mkdtemp(prefix="skillopt-sleep-copilot-cli-") - snapshot = os.path.join(tmpdir, "session-store.db") - shutil.copyfile(store_path, snapshot) - for suffix in ("-wal", "-shm"): - sidecar = store_path + suffix - if os.path.exists(sidecar): - shutil.copyfile(sidecar, snapshot + suffix) - return sqlite3.connect(_ro_uri(snapshot), uri=True, timeout=0), tmpdir + try: + snapshot = os.path.join(tmpdir, "session-store.db") + shutil.copyfile(store_path, snapshot) + for suffix in ("-wal", "-shm"): + sidecar = store_path + suffix + if os.path.exists(sidecar): + shutil.copyfile(sidecar, snapshot + suffix) + snap_con = sqlite3.connect(_ro_uri(snapshot), uri=True, timeout=0) + # Validate the snapshot schema too, so a later query cannot abort the run. + snap_con.execute("SELECT 1 FROM sessions LIMIT 1").fetchone() + return snap_con, tmpdir + except (sqlite3.Error, OSError): + shutil.rmtree(tmpdir, ignore_errors=True) + raise def harvest_copilot_cli( @@ -178,7 +190,8 @@ def harvest_copilot_cli( files = [ r["file_path"] for r in con.execute( - "SELECT DISTINCT file_path FROM session_files WHERE session_id = ?", + "SELECT DISTINCT file_path FROM session_files " + "WHERE session_id = ? ORDER BY file_path", (row["id"],), ) if r["file_path"] @@ -219,6 +232,9 @@ def harvest_copilot_cli( if limit and len(digests) >= limit: break return digests + except sqlite3.Error: + # Schema drift or mid-read corruption must not abort the run. + return [] finally: con.close() if tmpdir: diff --git a/tests/test_harvest_copilot_cli.py b/tests/test_harvest_copilot_cli.py index fd9a50dd..430f53f2 100644 --- a/tests/test_harvest_copilot_cli.py +++ b/tests/test_harvest_copilot_cli.py @@ -3,8 +3,6 @@ import os import sqlite3 -import pytest - from skillopt_sleep.harvest_copilot_cli import default_session_store, harvest_copilot_cli _SCHEMA = """ @@ -218,6 +216,22 @@ def test_since_iso_filters_on_session_end_not_start(tmp_path) -> None: assert ids == ["long"] +def test_query_error_mid_read_fails_closed(tmp_path) -> None: + # The sessions schema validates, but a missing turns table makes the + # per-session query raise mid-read; the harvest must yield [] rather than + # abort the run. + path = _store( + tmp_path, + [("s1", r"C:\p", "", "", "2026-01-01 10:00:00", "2026-01-01 10:30:00")], + [("s1", 0, "a real task", "ok", "2026-01-01 10:00:00")], + ) + con = sqlite3.connect(path) + con.execute("DROP TABLE turns") + con.commit() + con.close() + assert harvest_copilot_cli(path, scope="all") == [] + + def test_session_without_cwd_is_skipped(tmp_path) -> None: # No stable cwd -> not scopable and would collide on project+intent hashing. From 9b13be0301be40e4741894d554a2ae476372cd90 Mon Sep 17 00:00:00 2001 From: Christopher Haugen <1143916+lufen@users.noreply.github.com> Date: Wed, 5 Aug 2026 08:29:16 +0200 Subject: [PATCH 5/5] test(sleep): compare dispatched store against the normalized config value Re-review catch: SleepConfig.copilot_cli_session_store normalizes via os.path.abspath(os.path.expanduser(...)); on non-Windows the raw 'C:\\x\\store.db' is not absolute, so asserting equality with the raw string would fail on Linux (where the suite is re-run before merge). Compare against cfg.copilot_cli_session_store instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_harvest_copilot_cli.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_harvest_copilot_cli.py b/tests/test_harvest_copilot_cli.py index 430f53f2..b33a3ee4 100644 --- a/tests/test_harvest_copilot_cli.py +++ b/tests/test_harvest_copilot_cli.py @@ -198,7 +198,9 @@ def _fake(store, *, scope, invoked_project, since_iso, limit): out = harvest_sources.harvest_for_config(cfg, since_iso="2026-01-01", limit=5) assert out == ["digest"] - assert seen["store"] == r"C:\x\store.db" + # harvest_for_config passes the normalized config property (abspath/expanduser), + # so compare against that rather than the raw string to stay cross-platform. + assert seen["store"] == cfg.copilot_cli_session_store assert seen["scope"] == "all" assert seen["since_iso"] == "2026-01-01" assert seen["limit"] == 5