fix: address memory review findings

Remove shell-based memory tree inspection and pre-sync before memory pushes.
Switch sync update detection to git HEAD comparisons instead of localized output parsing.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Harivansh Rathi 2026-03-08 12:56:39 -07:00
parent 2886855706
commit e0801dedb0
3 changed files with 129 additions and 60 deletions

View file

@ -479,6 +479,15 @@ function getRepoName(settings: MemoryMdSettings): string {
return match?.[1] ?? "memory-md"; return match?.[1] ?? "memory-md";
} }
async function getGitHead(cwd: string): Promise<string | null> {
const result = await runGit(cwd, "rev-parse", "HEAD");
if (!result.success) {
return null;
}
const head = result.stdout.trim();
return head.length > 0 ? head : null;
}
async function getRepositoryDirtyState( async function getRepositoryDirtyState(
localPath: string, localPath: string,
projectPath?: string, projectPath?: string,
@ -541,6 +550,7 @@ async function syncRepository(
message: `Directory exists but is not a git repo: ${localPath}`, message: `Directory exists but is not a git repo: ${localPath}`,
}; };
} }
const previousHead = await getGitHead(localPath);
const pullResult = await runGit(localPath, "pull", "--rebase", "--autostash"); const pullResult = await runGit(localPath, "pull", "--rebase", "--autostash");
if (!pullResult.success) { if (!pullResult.success) {
return { return {
@ -549,14 +559,20 @@ async function syncRepository(
pullResult.stderr.trim() || "Pull failed. Check repository state.", pullResult.stderr.trim() || "Pull failed. Check repository state.",
}; };
} }
const currentHead = await getGitHead(localPath);
const updated = const updated =
pullResult.stdout.includes("Updating") || previousHead !== null &&
pullResult.stdout.includes("Fast-forward"); currentHead !== null &&
previousHead !== currentHead;
const message =
previousHead === null || currentHead === null
? `Synchronized ${getRepoName(settings)}`
: updated
? `Pulled latest changes from ${getRepoName(settings)}`
: `${getRepoName(settings)} is already up to date`;
return { return {
success: true, success: true,
message: updated message,
? `Pulled latest changes from ${getRepoName(settings)}`
: `${getRepoName(settings)} is already up to date`,
updated, updated,
}; };
} }
@ -861,17 +877,16 @@ export async function syncProjectMemory(
}; };
} }
if (!repositoryReady) { const syncResult = await syncRepository(settings);
const cloned = await syncRepository(settings); if (!syncResult.success) {
if (!cloned.success) { return {
return { success: false,
success: false, message: syncResult.message,
message: cloned.message, configured,
configured, initialized,
initialized, dirty: null,
dirty: null, updated: syncResult.updated,
}; };
}
} }
const statusResult = await runGit( const statusResult = await runGit(
@ -941,5 +956,6 @@ export async function syncProjectMemory(
initialized, initialized,
dirty: await getRepositoryDirtyState(localPath, projectPath), dirty: await getRepositoryDirtyState(localPath, projectPath),
committed: hasChanges, committed: hasChanges,
updated: syncResult.updated,
}; };
} }

View file

@ -147,6 +147,18 @@ function getRepoName(settings: MemoryMdSettings): string {
return match ? match[1] : "memory-md"; return match ? match[1] : "memory-md";
} }
async function getGitHead(
pi: ExtensionAPI,
cwd: string,
): Promise<string | null> {
const result = await gitExec(pi, cwd, "rev-parse", "HEAD");
if (!result.success) {
return null;
}
const head = result.stdout.trim();
return head.length > 0 ? head : null;
}
function loadScopedSettings(settingsPath: string): MemoryMdSettings { function loadScopedSettings(settingsPath: string): MemoryMdSettings {
if (!fs.existsSync(settingsPath)) { if (!fs.existsSync(settingsPath)) {
return {}; return {};
@ -286,6 +298,7 @@ export async function syncRepository(
}; };
} }
const previousHead = await getGitHead(pi, localPath);
const pullResult = await gitExec( const pullResult = await gitExec(
pi, pi,
localPath, localPath,
@ -301,15 +314,21 @@ export async function syncRepository(
} }
isRepoInitialized.value = true; isRepoInitialized.value = true;
const currentHead = await getGitHead(pi, localPath);
const updated = const updated =
pullResult.stdout.includes("Updating") || previousHead !== null &&
pullResult.stdout.includes("Fast-forward"); currentHead !== null &&
previousHead !== currentHead;
const repoName = getRepoName(settings); const repoName = getRepoName(settings);
const message =
previousHead === null || currentHead === null
? `Synchronized [${repoName}]`
: updated
? `Pulled latest changes from [${repoName}]`
: `[${repoName}] is already latest`;
return { return {
success: true, success: true,
message: updated message,
? `Pulled latest changes from [${repoName}]`
: `[${repoName}] is already latest`,
updated, updated,
}; };
} }
@ -477,6 +496,68 @@ export function createDefaultFiles(memoryDir: string): void {
} }
} }
export function formatMemoryDirectoryTree(
memoryDir: string,
maxDepth = 3,
maxLines = 40,
): string {
if (!fs.existsSync(memoryDir)) {
return "Unable to generate directory tree.";
}
const lines = [`${path.basename(memoryDir) || memoryDir}/`];
let truncated = false;
function visit(dir: string, depth: number, prefix: string): void {
if (depth >= maxDepth || lines.length >= maxLines) {
truncated = true;
return;
}
let entries: fs.Dirent[];
try {
entries = fs
.readdirSync(dir, { withFileTypes: true })
.filter((entry) => entry.name !== "node_modules")
.sort((left, right) => {
if (left.isDirectory() !== right.isDirectory()) {
return left.isDirectory() ? -1 : 1;
}
return left.name.localeCompare(right.name);
});
} catch {
truncated = true;
return;
}
for (const [index, entry] of entries.entries()) {
if (lines.length >= maxLines) {
truncated = true;
return;
}
const isLast = index === entries.length - 1;
const marker = isLast ? "\\-- " : "|-- ";
const childPrefix = `${prefix}${isLast ? " " : "| "}`;
lines.push(
`${prefix}${marker}${entry.name}${entry.isDirectory() ? "/" : ""}`,
);
if (entry.isDirectory()) {
visit(path.join(dir, entry.name), depth + 1, childPrefix);
}
}
}
visit(memoryDir, 0, "");
if (truncated) {
lines.push("... (tree truncated)");
}
return lines.join("\n");
}
function buildMemoryContext( function buildMemoryContext(
settings: MemoryMdSettings, settings: MemoryMdSettings,
ctx: ExtensionContext, ctx: ExtensionContext,
@ -779,25 +860,7 @@ export default function memoryMdExtension(pi: ExtensionAPI) {
return; return;
} }
const { execSync } = await import("node:child_process"); ctx.ui.notify(formatMemoryDirectoryTree(memoryDir).trim(), "info");
let treeOutput = "";
try {
treeOutput = execSync(`tree -L 3 -I "node_modules" "${memoryDir}"`, {
encoding: "utf-8",
});
} catch {
try {
treeOutput = execSync(
`find "${memoryDir}" -type d -not -path "*/node_modules/*"`,
{ encoding: "utf-8" },
);
} catch {
treeOutput = "Unable to generate directory tree.";
}
}
ctx.ui.notify(treeOutput.trim(), "info");
}, },
}); });
} }

View file

@ -8,6 +8,7 @@ import type { MemoryFrontmatter, MemoryMdSettings } from "./memory-md.js";
import { import {
createDefaultFiles, createDefaultFiles,
ensureDirectoryStructure, ensureDirectoryStructure,
formatMemoryDirectoryTree,
getCurrentDate, getCurrentDate,
getMemoryDir, getMemoryDir,
getProjectRepoPath, getProjectRepoPath,
@ -184,6 +185,14 @@ export function registerMemorySync(
}; };
} }
const syncResult = await syncRepository(pi, settings, isRepoInitialized);
if (!syncResult.success) {
return {
content: [{ type: "text", text: syncResult.message }],
details: { success: false, configured: true },
};
}
const statusResult = await gitExec( const statusResult = await gitExec(
pi, pi,
localPath, localPath,
@ -793,26 +802,7 @@ export function registerMemoryCheck(
}; };
} }
const { execSync } = await import("node:child_process"); const treeOutput = formatMemoryDirectoryTree(memoryDir);
let treeOutput = "";
try {
treeOutput = execSync(`tree -L 3 -I "node_modules" "${memoryDir}"`, {
encoding: "utf-8",
});
} catch {
try {
treeOutput = execSync(
`find "${memoryDir}" -type d -not -path "*/node_modules/*" | head -20`,
{
encoding: "utf-8",
},
);
} catch {
treeOutput =
"Unable to generate directory tree. Please check permissions.";
}
}
const files = listMemoryFiles(memoryDir); const files = listMemoryFiles(memoryDir);
const relPaths = files.map((f) => path.relative(memoryDir, f)); const relPaths = files.map((f) => path.relative(memoryDir, f));