Skip to content

feat(sleep): harvest GitHub Copilot CLI sessions - #200

Merged
Yif-Yang merged 5 commits into
microsoft:mainfrom
lufen:feat/copilot-cli-harvest
Aug 6, 2026
Merged

feat(sleep): harvest GitHub Copilot CLI sessions#200
Yif-Yang merged 5 commits into
microsoft:mainfrom
lufen:feat/copilot-cli-harvest

Conversation

@lufen

@lufen lufen commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

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 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.
Copilot AI lite review requested due to automatic review settings August 4, 2026 13:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new transcript source to SkillOpt-Sleep that harvests GitHub Copilot CLI sessions directly from the CLI’s global SQLite session store, enabling mining of substantially more sessions/turns than VS Code transcripts and avoiding expensive per-session log parsing.

Changes:

  • Introduces harvest_copilot_cli to read ~/.copilot/session-store.db in read-only mode with snapshot fallback and self-call filtering.
  • Registers copilot_cli as a selectable source in config + CLI flags and routes selection via harvest_for_config.
  • Adds unit tests covering mapping, scoping, date filtering, limiting, and self-call filtering for the new source.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/test_harvest_copilot_cli.py Adds test coverage for the Copilot CLI SQLite harvester behavior.
skillopt_sleep/harvest.py Extends replay/meta prompt markers to better filter engine/self-generated prompts.
skillopt_sleep/harvest_sources.py Registers and routes the new copilot_cli harvest source.
skillopt_sleep/harvest_copilot_cli.py Implements the Copilot CLI SQLite session harvester with snapshot fallback and normalization into SessionDigest.
skillopt_sleep/config.py Adds copilot_cli_session_store config knob (and default).
skillopt_sleep/main.py Adds CLI selection/override flags for copilot_cli harvesting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skillopt_sleep/harvest_copilot_cli.py Outdated
Comment thread skillopt_sleep/harvest_copilot_cli.py Outdated
Comment thread skillopt_sleep/harvest_copilot_cli.py Outdated
Comment thread skillopt_sleep/harvest_copilot_cli.py Outdated
Comment thread tests/test_harvest_copilot_cli.py Outdated
- 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>
Copilot AI review requested due to automatic review settings August 4, 2026 16:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (6)

skillopt_sleep/harvest_copilot_cli.py:96

  • The snapshot connection should also use timeout=0 for the same "must never block" guarantee (otherwise a locked snapshot copy can still stall the harvest).
    return sqlite3.connect(_ro_uri(snapshot), uri=True), tmpdir

skillopt_sleep/harvest_copilot_cli.py:126

  • since_iso is documented/used elsewhere as a cutoff on session end time, but this query filters by created_at. That will drop sessions that started before the cutoff but ended after it (especially long-lived CLI sessions). Filter on updated_at instead to match the "sessions ending after" semantics used in other harvesters.
        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)

skillopt_sleep/harvest_copilot_cli.py:131

  • con.execute(...).fetchall() loads all session rows into memory even when limit is small. Iterating the cursor directly avoids unnecessary memory/time and keeps the same behavior with the existing early break.
        rows = con.execute(
            "SELECT id, cwd, repository, branch, created_at, updated_at "
            f"FROM sessions {where} ORDER BY updated_at DESC",
            params,
        ).fetchall()

skillopt_sleep/harvest_copilot_cli.py:197

  • Other harvesters consistently keep only the last few assistant finals (see harvest.py and harvest_copilot.py) because only the final answer is used downstream. Keeping up to 40 full assistant messages here unnecessarily increases harvested data size/token pressure without affecting mining behavior (which uses assistant_finals[-1]).
                    assistant_finals=finals,

skillopt_sleep/harvest_copilot_cli.py:83

  • sqlite3.connect uses a default busy timeout (typically 5s). Since the PR description requires harvesting to never block live CLI sessions, set timeout=0 for the read-only open so lock contention fails fast and the code can fall back to the snapshot path.

This issue also appears on line 96 of the same file.

        con = sqlite3.connect(_ro_uri(store_path), uri=True)

tests/test_harvest_copilot_cli.py:188

  • test_source_is_registered currently asserts on raw source text, which is brittle and can pass even if the runtime dispatch is broken (e.g., the string appears in a comment). Prefer asserting behavior by monkeypatching harvest_sources.harvest_copilot_cli and verifying harvest_for_config() calls it when transcript_source="copilot_cli".
@pytest.mark.parametrize("source", ["copilot_cli"])
def test_source_is_registered(source: str) -> None:
    from skillopt_sleep import harvest_sources

    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

- 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>
Copilot AI review requested due to automatic review settings August 4, 2026 17:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

skillopt_sleep/harvest_copilot_cli.py:184

  • The files_touched list is built from a SELECT DISTINCT without ORDER BY, so its ordering can be nondeterministic across SQLite versions/plans. Since SessionDigest is persisted/hashed downstream, make the ordering deterministic (e.g., ORDER BY file_path).
                for r in con.execute(
                    "SELECT DISTINCT file_path FROM session_files WHERE session_id = ?",
                    (row["id"],),
                )
                if r["file_path"]

skillopt_sleep/harvest_copilot_cli.py:98

  • _connect() can leak the initial SQLite connection on the read-only open failure path, and the snapshot path doesn’t validate the schema (so later queries can raise sqlite3.Error and abort the run). Also, if snapshotting fails after tmpdir creation, the temp dir is never cleaned up.
    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

tests/test_harvest_copilot_cli.py:8

  • tests/test_harvest_copilot_cli.py imports pytest but never uses it; ruff is configured to error on unused imports (F401). Remove the unused import to keep lint passing.
import pytest

skillopt_sleep/harvest_copilot_cli.py:225

  • harvest_copilot_cli() still allows sqlite3.Error exceptions from queries (e.g., schema drift/corruption) to propagate and abort the run, despite the “must never block or abort” requirement. Catch sqlite3.Error around the main query loop and fail closed (return []).
        return digests
    finally:
        con.close()

- 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>
Copilot AI review requested due to automatic review settings August 5, 2026 06:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tests/test_harvest_copilot_cli.py:201

  • harvest_for_config() passes cfg.copilot_cli_session_store, which is normalized by the SleepConfig.copilot_cli_session_store property via os.path.abspath(os.path.expanduser(...)). On non-Windows platforms, r"C:\x\store.db" is not absolute, so seen["store"] will not equal the raw string and this assertion will fail.
    assert seen["store"] == r"C:\x\store.db"

@lufen

lufen commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up in 331d6d7 addresses the re-review's suppressed suggestions: removed an unused pytest import (F401), added ORDER BY file_path for deterministic files_touched, hardened _connect (close the half-open connection on the read-only failure path, validate the snapshot schema, clean up the temp dir if snapshotting fails), and wrapped the main query loop to fail closed (return []) on sqlite3.Error mid-read. ruff clean; all suites green.

…alue

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>
Copilot AI review requested due to automatic review settings August 5, 2026 06:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@lufen

lufen commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Second re-review follow-up in 9b13be0: the new dispatch test compared seen["store"] against a raw C:\x\store.db, but SleepConfig.copilot_cli_session_store normalizes via os.path.abspath, so that assertion would fail on Linux (where the suite is re-run before merge). It now compares against cfg.copilot_cli_session_store. Suites green on Windows.

@Yif-Yang Yif-Yang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated the opt-in Copilot CLI harvester, read-only/fail-closed behavior, self-call filtering, and tests. This adds a useful real-world transcript source. We will immediately add shared secret redaction at the transcript-to-miner boundary as a maintainer hardening follow-up.

@Yif-Yang
Yif-Yang merged commit 7c52718 into microsoft:main Aug 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants