fix(dream): filter non-Dream history commits (#4905)
* fix(dream): filter non-Dream history commits * fix(dream): resolve filtered commit diffs
This commit is contained in:
+27
-10
@@ -560,6 +560,9 @@ def _format_changed_files(diff: str) -> str:
|
||||
return ", ".join(f"`{path}`" for path in files)
|
||||
|
||||
|
||||
_DREAM_COMMIT_PREFIX = "dream:"
|
||||
|
||||
|
||||
def _format_dream_log_content(commit, diff: str, *, requested_sha: str | None = None) -> str:
|
||||
files_line = _format_changed_files(diff)
|
||||
lines = [
|
||||
@@ -608,7 +611,7 @@ def _format_dream_restore_list(commits: list) -> str:
|
||||
async def cmd_dream_log(ctx: CommandContext) -> OutboundMessage:
|
||||
"""Show what the last Dream changed.
|
||||
|
||||
Default: diff of the latest commit (HEAD~1 vs HEAD).
|
||||
Default: diff of the latest Dream commit versus its parent.
|
||||
With /dream-log <sha>: diff of that specific commit.
|
||||
"""
|
||||
store = ctx.loop.consolidator.store
|
||||
@@ -643,9 +646,16 @@ async def cmd_dream_log(ctx: CommandContext) -> OutboundMessage:
|
||||
commit, diff = result
|
||||
content = _format_dream_log_content(commit, diff, requested_sha=sha)
|
||||
else:
|
||||
# Default: show the latest commit's diff
|
||||
commits = git.log(max_entries=1)
|
||||
result = git.show_commit_diff(commits[0].sha) if commits else None
|
||||
# Default: show the latest Dream commit's diff
|
||||
commits = git.log(max_entries=1, message_prefix=_DREAM_COMMIT_PREFIX)
|
||||
result = (
|
||||
git.show_commit_diff(
|
||||
commits[0].sha,
|
||||
max_entries=1,
|
||||
message_prefix=_DREAM_COMMIT_PREFIX,
|
||||
)
|
||||
if commits else None
|
||||
)
|
||||
if result:
|
||||
commit, diff = result
|
||||
content = _format_dream_log_content(commit, diff)
|
||||
@@ -678,17 +688,24 @@ async def cmd_dream_restore(ctx: CommandContext) -> OutboundMessage:
|
||||
|
||||
args = ctx.args.strip()
|
||||
if not args:
|
||||
# Show recent commits for the user to pick
|
||||
commits = git.log(max_entries=10)
|
||||
# Show recent Dream commits for the user to pick
|
||||
commits = git.log(max_entries=10, message_prefix=_DREAM_COMMIT_PREFIX)
|
||||
if not commits:
|
||||
content = "Dream memory has no saved versions to restore yet."
|
||||
else:
|
||||
content = _format_dream_restore_list(commits)
|
||||
else:
|
||||
sha = args.split()[0]
|
||||
result = git.show_commit_diff(sha)
|
||||
changed_files = _format_changed_files(result[1]) if result else "the tracked memory files"
|
||||
new_sha = git.revert(sha)
|
||||
result = git.show_commit_diff(sha, message_prefix=_DREAM_COMMIT_PREFIX)
|
||||
if not result:
|
||||
content = (
|
||||
f"Couldn't restore Dream change `{sha}`.\n\n"
|
||||
"Only Dream memory versions can be restored. "
|
||||
"Use `/dream-restore` to list recent versions."
|
||||
)
|
||||
else:
|
||||
changed_files = _format_changed_files(result[1])
|
||||
new_sha = git.revert(sha, message_prefix=_DREAM_COMMIT_PREFIX)
|
||||
if new_sha:
|
||||
content = (
|
||||
f"Restored Dream memory to the state before `{sha}`.\n\n"
|
||||
@@ -699,7 +716,7 @@ async def cmd_dream_restore(ctx: CommandContext) -> OutboundMessage:
|
||||
else:
|
||||
content = (
|
||||
f"Couldn't restore Dream change `{sha}`.\n\n"
|
||||
"It may not exist, or it may be the first saved version with no earlier state to restore."
|
||||
"It may be the first saved version with no earlier state to restore."
|
||||
)
|
||||
return OutboundMessage(
|
||||
channel=ctx.msg.channel, chat_id=ctx.msg.chat_id,
|
||||
|
||||
+46
-12
@@ -214,8 +214,16 @@ class GitStore:
|
||||
|
||||
# -- query -----------------------------------------------------------------
|
||||
|
||||
def log(self, max_entries: int = 20) -> list[CommitInfo]:
|
||||
"""Return simplified commit log."""
|
||||
def log(
|
||||
self,
|
||||
max_entries: int = 20,
|
||||
message_prefix: str | None = None,
|
||||
) -> list[CommitInfo]:
|
||||
"""Return simplified commit log, optionally filtered by message prefix.
|
||||
|
||||
When filtering, *max_entries* counts matching commits rather than every
|
||||
commit traversed in the repository.
|
||||
"""
|
||||
if not self.is_initialized():
|
||||
return []
|
||||
|
||||
@@ -239,6 +247,7 @@ class GitStore:
|
||||
time.localtime(commit.commit_time),
|
||||
)
|
||||
msg = commit.message.decode("utf-8", errors="replace").strip()
|
||||
if message_prefix is None or msg.startswith(message_prefix):
|
||||
entries.append(CommitInfo(
|
||||
sha=sha.hex()[:8],
|
||||
message=msg,
|
||||
@@ -424,25 +433,41 @@ class GitStore:
|
||||
return c
|
||||
return None
|
||||
|
||||
def show_commit_diff(self, short_sha: str, max_entries: int = 20) -> tuple[CommitInfo, str] | None:
|
||||
"""Find a commit and return it with its diff vs the parent."""
|
||||
commits = self.log(max_entries=max_entries)
|
||||
for i, c in enumerate(commits):
|
||||
def show_commit_diff(
|
||||
self,
|
||||
short_sha: str,
|
||||
max_entries: int = 20,
|
||||
message_prefix: str | None = None,
|
||||
) -> tuple[CommitInfo, str] | None:
|
||||
"""Find a commit and return it with its diff vs its actual parent."""
|
||||
try:
|
||||
from dulwich.repo import Repo
|
||||
|
||||
commits = self.log(max_entries=max_entries, message_prefix=message_prefix)
|
||||
for c in commits:
|
||||
if c.sha.startswith(short_sha):
|
||||
if i + 1 < len(commits):
|
||||
diff = self.diff_commits(commits[i + 1].sha, c.sha)
|
||||
else:
|
||||
diff = ""
|
||||
full_sha = self._resolve_sha(c.sha)
|
||||
if not full_sha:
|
||||
return None
|
||||
with Repo(str(self._workspace)) as repo:
|
||||
commit = repo[full_sha]
|
||||
parent = commit.parents[0] if commit.parents else None
|
||||
diff = self.diff_commits(parent.hex()[:8], c.sha) if parent else ""
|
||||
return c, diff
|
||||
return None
|
||||
except Exception:
|
||||
logger.exception("Git show_commit_diff failed")
|
||||
return None
|
||||
|
||||
# -- restore ---------------------------------------------------------------
|
||||
|
||||
def revert(self, commit: str) -> str | None:
|
||||
def revert(self, commit: str, *, message_prefix: str | None = None) -> str | None:
|
||||
"""Revert (undo) the changes introduced by the given commit.
|
||||
|
||||
Restores all tracked memory files to the state at the commit's parent,
|
||||
then creates a new commit recording the revert.
|
||||
then creates a new commit recording the revert. When *message_prefix*
|
||||
is provided, commits outside that history are rejected before any files
|
||||
are changed.
|
||||
|
||||
Returns the new commit SHA, or None on failure.
|
||||
"""
|
||||
@@ -462,6 +487,15 @@ class GitStore:
|
||||
if commit_obj.type_name != b"commit":
|
||||
return None
|
||||
|
||||
commit_message = commit_obj.message.decode("utf-8", errors="replace").strip()
|
||||
if message_prefix is not None and not commit_message.startswith(message_prefix):
|
||||
logger.warning(
|
||||
"Git revert: commit {} does not match message prefix {!r}",
|
||||
commit,
|
||||
message_prefix,
|
||||
)
|
||||
return None
|
||||
|
||||
if not commit_obj.parents:
|
||||
logger.warning("Git revert: cannot revert root commit {}", commit)
|
||||
return None
|
||||
|
||||
@@ -117,6 +117,17 @@ class TestLog:
|
||||
git_ready.auto_commit(f"c{i}")
|
||||
assert len(git_ready.log(max_entries=3)) == 3
|
||||
|
||||
def test_message_prefix_skips_unrelated_commits_before_counting_limit(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
messages = ["dream: older", "backup: first", "dream: latest", "backup: newest"]
|
||||
for i, message in enumerate(messages):
|
||||
(ws / "SOUL.md").write_text(f"v{i}", encoding="utf-8")
|
||||
git_ready.auto_commit(message)
|
||||
|
||||
commits = git_ready.log(max_entries=2, message_prefix="dream:")
|
||||
|
||||
assert [commit.message for commit in commits] == ["dream: latest", "dream: older"]
|
||||
|
||||
def test_commit_info_fields(self, git_ready):
|
||||
c = git_ready.log()[0]
|
||||
assert isinstance(c, CommitInfo)
|
||||
@@ -178,6 +189,25 @@ class TestShowCommitDiff:
|
||||
def test_returns_none_for_unknown(self, git_ready):
|
||||
assert git_ready.show_commit_diff("deadbeef") is None
|
||||
|
||||
def test_message_prefix_finds_commit_beyond_unrelated_history_window(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
(ws / "SOUL.md").write_text("dream content", encoding="utf-8")
|
||||
dream_sha = git_ready.auto_commit("dream: latest")
|
||||
for i in range(20):
|
||||
(ws / "SOUL.md").write_text(f"backup {i}", encoding="utf-8")
|
||||
git_ready.auto_commit(f"backup: {i}")
|
||||
|
||||
result = git_ready.show_commit_diff(
|
||||
dream_sha,
|
||||
max_entries=1,
|
||||
message_prefix="dream:",
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
commit, diff = result
|
||||
assert commit.sha == dream_sha
|
||||
assert "dream content" in diff
|
||||
|
||||
|
||||
class TestCommitInfoFormat:
|
||||
def test_format_with_diff(self):
|
||||
@@ -221,6 +251,19 @@ class TestRevert:
|
||||
def test_invalid_sha_returns_none(self, git_ready):
|
||||
assert git_ready.revert("deadbeef") is None
|
||||
|
||||
def test_message_prefix_rejects_unrelated_commit_without_changing_files(self, git_ready):
|
||||
ws = git_ready._workspace
|
||||
(ws / "SOUL.md").write_text("dream v1", encoding="utf-8")
|
||||
git_ready.auto_commit("dream: v1")
|
||||
(ws / "SOUL.md").write_text("backup state", encoding="utf-8")
|
||||
backup_sha = git_ready.auto_commit("backup: workspace")
|
||||
(ws / "SOUL.md").write_text("dream v2", encoding="utf-8")
|
||||
latest_sha = git_ready.auto_commit("dream: v2")
|
||||
|
||||
assert git_ready.revert(backup_sha, message_prefix="dream:") is None
|
||||
assert (ws / "SOUL.md").read_text(encoding="utf-8") == "dream v2"
|
||||
assert git_ready.log()[0].sha == latest_sha
|
||||
|
||||
|
||||
class TestMemoryStoreGitProperty:
|
||||
def test_git_property_exposes_gitstore(self, tmp_path):
|
||||
|
||||
@@ -65,17 +65,34 @@ class _FakeGit:
|
||||
self._commits = commits or []
|
||||
self._diff_map = diff_map or {}
|
||||
self._revert_result = revert_result
|
||||
self.revert_calls: list[tuple[str, str | None]] = []
|
||||
|
||||
def is_initialized(self) -> bool:
|
||||
return self._initialized
|
||||
|
||||
def log(self, max_entries: int = 20) -> list[CommitInfo]:
|
||||
return self._commits[:max_entries]
|
||||
def log(
|
||||
self,
|
||||
max_entries: int = 20,
|
||||
message_prefix: str | None = None,
|
||||
) -> list[CommitInfo]:
|
||||
commits = self._commits
|
||||
if message_prefix is not None:
|
||||
commits = [c for c in commits if c.message.startswith(message_prefix)]
|
||||
return commits[:max_entries]
|
||||
|
||||
def show_commit_diff(self, sha: str, max_entries: int = 20):
|
||||
return self._diff_map.get(sha)
|
||||
def show_commit_diff(
|
||||
self,
|
||||
sha: str,
|
||||
max_entries: int = 20,
|
||||
message_prefix: str | None = None,
|
||||
):
|
||||
result = self._diff_map.get(sha)
|
||||
if result and message_prefix is not None and not result[0].message.startswith(message_prefix):
|
||||
return None
|
||||
return result
|
||||
|
||||
def revert(self, sha: str) -> str | None:
|
||||
def revert(self, sha: str, *, message_prefix: str | None = None) -> str | None:
|
||||
self.revert_calls.append((sha, message_prefix))
|
||||
return self._revert_result
|
||||
|
||||
def auto_commit(self, message: str) -> str | None:
|
||||
@@ -260,6 +277,26 @@ async def test_dream_log_latest_is_more_user_friendly() -> None:
|
||||
assert "```diff" in out.content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dream_log_latest_skips_non_dream_commit() -> None:
|
||||
backup = CommitInfo(
|
||||
sha="bbbb2222", message="backup: workspace snapshot", timestamp="2026-04-04 13:00",
|
||||
)
|
||||
dream = CommitInfo(
|
||||
sha="abcd1234", message="dream: latest", timestamp="2026-04-04 12:00",
|
||||
)
|
||||
diff = "diff --git a/SOUL.md b/SOUL.md\n"
|
||||
git = _FakeGit(
|
||||
commits=[backup, dream],
|
||||
diff_map={dream.sha: (dream, diff), backup.sha: (backup, "unrelated diff")},
|
||||
)
|
||||
|
||||
out = await cmd_dream_log(_make_ctx("/dream-log", git))
|
||||
|
||||
assert "`abcd1234`" in out.content
|
||||
assert "`bbbb2222`" not in out.content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dream_log_missing_commit_guides_user() -> None:
|
||||
git = _FakeGit(diff_map={})
|
||||
@@ -357,6 +394,7 @@ def test_dream_prompt_command_in_help_and_palette() -> None:
|
||||
async def test_dream_restore_lists_versions_with_next_steps() -> None:
|
||||
commits = [
|
||||
CommitInfo(sha="abcd1234", message="dream: latest", timestamp="2026-04-04 12:00"),
|
||||
CommitInfo(sha="cccc3333", message="backup: workspace", timestamp="2026-04-04 10:00"),
|
||||
CommitInfo(sha="bbbb2222", message="dream: older", timestamp="2026-04-04 08:00"),
|
||||
]
|
||||
git = _FakeGit(commits=commits)
|
||||
@@ -366,6 +404,8 @@ async def test_dream_restore_lists_versions_with_next_steps() -> None:
|
||||
assert "## Dream Restore" in out.content
|
||||
assert "Choose a Dream memory version to restore." in out.content
|
||||
assert "`abcd1234` 2026-04-04 12:00 - dream: latest" in out.content
|
||||
assert "`bbbb2222` 2026-04-04 08:00 - dream: older" in out.content
|
||||
assert "backup: workspace" not in out.content
|
||||
assert "Preview a version with `/dream-log <sha>`" in out.content
|
||||
assert "Restore a version with `/dream-restore <sha>`." in out.content
|
||||
|
||||
@@ -398,3 +438,21 @@ async def test_dream_restore_success_mentions_files_and_followup() -> None:
|
||||
assert "- New safety commit: `eeee9999`" in out.content
|
||||
assert "- Restored files: `SOUL.md`, `memory/MEMORY.md`" in out.content
|
||||
assert "Use `/dream-log eeee9999` to inspect the restore diff." in out.content
|
||||
assert git.revert_calls == [("abcd1234", "dream:")]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dream_restore_rejects_non_dream_commit_clearly() -> None:
|
||||
commit = CommitInfo(
|
||||
sha="cccc3333", message="backup: workspace", timestamp="2026-04-04 10:00",
|
||||
)
|
||||
git = _FakeGit(
|
||||
diff_map={commit.sha: (commit, "unrelated diff")},
|
||||
revert_result="eeee9999",
|
||||
)
|
||||
|
||||
out = await cmd_dream_restore(_make_ctx("/dream-restore cccc3333", git, args="cccc3333"))
|
||||
|
||||
assert "Only Dream memory versions can be restored." in out.content
|
||||
assert "Use `/dream-restore` to list recent versions." in out.content
|
||||
assert git.revert_calls == []
|
||||
|
||||
Reference in New Issue
Block a user