Conversation
Since git-mem is now the sole storage backend, removed: - Storage mode prompts from `lisa init` (local/zep-cloud/skip) - `--mode`, `--endpoint`, `--group`, `--zep-api-key`, `--zep-project-id` options - `lisa up` and `lisa down` commands - Docker, Neo4j, Zep connectivity checks from `lisa doctor` - `docker.ts` module and `.lisa/docker/` templates - `DeploymentMode`, `IGraphitiConfig` types and related constants - Deprecated settings from `.env.template` Updates tests to reflect simplified CLI interface. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughRemoves Docker/Zep deployment options and group-id scaffolding; migrates CLI init/doctor flows to a git-backed memory model (git notes / "git-mem"); deletes Docker compose templates and related env vars; updates exports and tests; bumps package to 2.27.0. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/cli.ts (1)
31-39:⚠️ Potential issue | 🟡 MinorStale comment references removed module.
The comment on line 34 still references
upCommand, downCommand -> docker.ts, but these commands and the module have been removed in this PR.🧹 Proposed fix to remove stale reference
// Command implementations moved to src/lib/commands/ // - initCommand, cleanupPreviousInstall -> init.ts // - doctorCommand -> doctor.ts -// - upCommand, downCommand -> docker.ts // - hook commands -> hooks.ts // - memory, tasks, storage -> knowledge.ts // - jira, github, prompt, etc. -> skills.ts // - issue create, labels -> issue.ts // - pr commands -> pr.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/cli.ts` around lines 31 - 39, The inline comment block in src/lib/cli.ts still lists "upCommand, downCommand -> docker.ts" which is stale; remove that bullet (or replace it with the current module name if those commands were moved elsewhere) so the comment accurately reflects the new locations (e.g., init.ts, doctor.ts, hooks.ts, knowledge.ts, skills.ts, issue.ts, pr.ts), and scan for any other lingering "docker" references in that comment to keep the summary consistent with the actual command files.tests/unit/src/cli.test.ts (1)
22-37: 🧹 Nitpick | 🔵 TrivialRemove unused
MockDockerClientmock class.
MockDockerClientis instantiated but never exercised in the test file. Its methods (version(),composeVersion(),compose()) and tracking variables are not called or asserted. ThedoctorCommandtest explicitly notes that "Doctor no longer checks Docker," confirming the mock is no longer needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/src/cli.test.ts` around lines 22 - 37, Remove the unused MockDockerClient class definition from the test file: delete the MockDockerClient class (its methods version, composeVersion, compose and composeCalls/versionCalls/composeVersionCalls fields) since it's instantiated but never exercised or asserted; also ensure no remaining references to MockDockerClient (e.g., in any doctorCommand-related setup) remain in the test so tests compile cleanly.src/lib/commands/doctor.ts (1)
84-97: 🧹 Nitpick | 🔵 TrivialConsider consolidating with the shared
getProjectNamefunction.This function duplicates logic from
src/lib/commands/shared/constants.ts. The difference is this version acceptscwdas a parameter while the shared version usesprocess.cwd().Consider refactoring the shared function to accept an optional
cwdparameter and reusing it here:// In shared/constants.ts export function getProjectName(cwd: string = process.cwd()): string { ... }Then import and use it in this file instead of maintaining duplicate implementations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/commands/doctor.ts` around lines 84 - 97, The local getProjectName function duplicates logic already implemented in the shared getProjectName; refactor the shared function (getProjectName in shared/constants.ts) to accept an optional cwd parameter (defaulting to process.cwd()), replace the local implementation in src/lib/commands/doctor.ts with an import of the shared getProjectName, and call it with the existing cwd argument; remove the duplicate function from doctor.ts to keep a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/commands/doctor.ts`:
- Around line 258-281: checkGitMem currently only checks the loose ref path
(notesRef) and can miss refs stored in .git/packed-refs; update checkGitMem to
also detect packed notes by either (a) reading and parsing the .git/packed-refs
file for a line containing "refs/notes/mem" when notesRef is absent, or (b)
prefer running a git command (e.g., "git notes list" or "git rev-parse
refs/notes/mem") from within checkGitMem and treat a successful result as
"Memory notes found"; keep the same returned shape (name, status, message,
details, durationMs) and use the existing start timestamp to compute durationMs.
---
Outside diff comments:
In `@src/lib/cli.ts`:
- Around line 31-39: The inline comment block in src/lib/cli.ts still lists
"upCommand, downCommand -> docker.ts" which is stale; remove that bullet (or
replace it with the current module name if those commands were moved elsewhere)
so the comment accurately reflects the new locations (e.g., init.ts, doctor.ts,
hooks.ts, knowledge.ts, skills.ts, issue.ts, pr.ts), and scan for any other
lingering "docker" references in that comment to keep the summary consistent
with the actual command files.
In `@src/lib/commands/doctor.ts`:
- Around line 84-97: The local getProjectName function duplicates logic already
implemented in the shared getProjectName; refactor the shared function
(getProjectName in shared/constants.ts) to accept an optional cwd parameter
(defaulting to process.cwd()), replace the local implementation in
src/lib/commands/doctor.ts with an import of the shared getProjectName, and call
it with the existing cwd argument; remove the duplicate function from doctor.ts
to keep a single source of truth.
In `@tests/unit/src/cli.test.ts`:
- Around line 22-37: Remove the unused MockDockerClient class definition from
the test file: delete the MockDockerClient class (its methods version,
composeVersion, compose and composeCalls/versionCalls/composeVersionCalls
fields) since it's instantiated but never exercised or asserted; also ensure no
remaining references to MockDockerClient (e.g., in any doctorCommand-related
setup) remain in the test so tests compile cleanly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
CHANGELOG.mdpackage.jsonsrc/lib/cli.tssrc/lib/commands/docker.tssrc/lib/commands/doctor.tssrc/lib/commands/index.tssrc/lib/commands/init.tssrc/lib/commands/shared/constants.tssrc/lib/commands/shared/index.tssrc/project/.lisa/.env.templatesrc/project/.lisa/docker/docker-compose.graphiti.ymltests/integration/cli/index.tstests/unit/src/cli.test.ts
💤 Files with no reviewable changes (4)
- src/project/.lisa/.env.template
- src/lib/commands/shared/index.ts
- src/lib/commands/docker.ts
- src/project/.lisa/docker/docker-compose.graphiti.yml
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e746978279
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/lib/commands/doctor.ts
Outdated
Comment on lines
+260
to
+263
| const notesRef = path.join(cwd, '.git', 'refs', 'notes', 'mem'); | ||
|
|
||
| // Check if git notes for memory exist | ||
| const hasNotes = await fs.pathExists(notesRef); |
There was a problem hiding this comment.
checkGitMem treats .git/refs/notes/mem existence as the source of truth, but refs can be packed and still valid; git pack-refs -h documents --[no-]prune with “prune loose refs (default)”, so this file path can disappear while notes remain readable via git notes --ref=mem. That makes lisa doctor incorrectly report “no memories stored yet” for repositories that have packed note refs (and similarly for other non-loose ref layouts).
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in commit 0beb893 - now checks both loose refs and packed-refs. See the reply to CodeRabbit's similar comment above.
Update doctor tests to match simplified implementation: - Remove tests for deprecated Docker, mode, group, endpoint config - Add tests for Git Repository check - Add tests for Git-Mem Storage check - Update IConfigInfo expectations (storage, projectName only) - Update createMockResult to match new check types Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address review feedback: git notes can be packed by `git gc` into .git/packed-refs. Now checkGitMem checks both loose refs and packed-refs to accurately detect existing memories. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/commands/doctor.ts`:
- Around line 259-294: checkGitMem currently returns status 'ok' even when the
repository has no .git directory which contradicts the overall repo checks;
update the function checkGitMem to first verify the existence of the .git
directory (e.g., check path.join(cwd, '.git')) and if missing return a failing
ICheckResult (status: 'fail') with a clear message like "No .git repository
found — initialize a git repo" and appropriate durationMs; otherwise proceed
with the existing logic that inspects refs/notes/mem and packed-refs and return
'ok' only when the git metadata is present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
lisa init(local Docker, Zep Cloud, skip)--mode,--endpoint,--group,--zep-api-key,--zep-project-idlisa upandlisa downcommands (Docker Compose)lisa doctorto check git-mem and Lisa structure onlyContext
With git-mem now the sole storage backend (LISA-34, LISA-59), the Docker/Neo4j/Zep options were deprecated. This PR cleans them up.
Test plan
npm run buildsucceedsnpm run lintpasses (warnings are pre-existing)lisa --helpshows clean CLI without up/down commandslisa init --helpshows simplified optionslisa doctorruns successfully🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Removed Features
Improvements
Tests