Address Greptile review fixes

This commit is contained in:
Dotta 2026-03-15 06:13:50 -05:00
parent 269dd6abbe
commit 5de5fb507a
3 changed files with 39 additions and 22 deletions

View file

@ -1,6 +1,7 @@
import { promises as fs } from "node:fs"; import { promises as fs } from "node:fs";
import { execFileSync } from "node:child_process"; import { execFile } from "node:child_process";
import path from "node:path"; import path from "node:path";
import { promisify } from "node:util";
import type { Db } from "@paperclipai/db"; import type { Db } from "@paperclipai/db";
import type { import type {
CompanyPortabilityAgentManifestEntry, CompanyPortabilityAgentManifestEntry,
@ -47,6 +48,8 @@ const DEFAULT_INCLUDE: CompanyPortabilityInclude = {
}; };
const DEFAULT_COLLISION_STRATEGY: CompanyPortabilityCollisionStrategy = "rename"; const DEFAULT_COLLISION_STRATEGY: CompanyPortabilityCollisionStrategy = "rename";
const execFileAsync = promisify(execFile);
let bundledSkillsCommitPromise: Promise<string | null> | null = null;
function isSensitiveEnvKey(key: string) { function isSensitiveEnvKey(key: string) {
const normalized = key.trim().toLowerCase(); const normalized = key.trim().toLowerCase();
@ -603,19 +606,22 @@ function buildMarkdown(frontmatter: Record<string, unknown>, body: string) {
return `${renderFrontmatter(frontmatter)}\n${cleanBody}\n`; return `${renderFrontmatter(frontmatter)}\n${cleanBody}\n`;
} }
function buildSkillSourceEntry(skill: CompanySkill) { async function resolveBundledSkillsCommit() {
if (!bundledSkillsCommitPromise) {
bundledSkillsCommitPromise = execFileAsync("git", ["rev-parse", "HEAD"], {
cwd: process.cwd(),
encoding: "utf8",
})
.then(({ stdout }) => stdout.trim() || null)
.catch(() => null);
}
return bundledSkillsCommitPromise;
}
async function buildSkillSourceEntry(skill: CompanySkill) {
const metadata = isPlainRecord(skill.metadata) ? skill.metadata : null; const metadata = isPlainRecord(skill.metadata) ? skill.metadata : null;
if (asString(metadata?.sourceKind) === "paperclip_bundled") { if (asString(metadata?.sourceKind) === "paperclip_bundled") {
let commit: string | null = null; const commit = await resolveBundledSkillsCommit();
try {
const resolved = execFileSync("git", ["rev-parse", "HEAD"], {
cwd: process.cwd(),
encoding: "utf8",
}).trim();
commit = resolved || null;
} catch {
commit = null;
}
return { return {
kind: "github-dir", kind: "github-dir",
repo: "paperclipai/paperclip", repo: "paperclipai/paperclip",
@ -658,8 +664,8 @@ function shouldReferenceSkillOnExport(skill: CompanySkill, expandReferencedSkill
return skill.sourceType === "github" || skill.sourceType === "url"; return skill.sourceType === "github" || skill.sourceType === "url";
} }
function buildReferencedSkillMarkdown(skill: CompanySkill) { async function buildReferencedSkillMarkdown(skill: CompanySkill) {
const sourceEntry = buildSkillSourceEntry(skill); const sourceEntry = await buildSkillSourceEntry(skill);
const frontmatter: Record<string, unknown> = { const frontmatter: Record<string, unknown> = {
name: skill.name, name: skill.name,
description: skill.description ?? null, description: skill.description ?? null,
@ -672,8 +678,8 @@ function buildReferencedSkillMarkdown(skill: CompanySkill) {
return buildMarkdown(frontmatter, ""); return buildMarkdown(frontmatter, "");
} }
function withSkillSourceMetadata(skill: CompanySkill, markdown: string) { async function withSkillSourceMetadata(skill: CompanySkill, markdown: string) {
const sourceEntry = buildSkillSourceEntry(skill); const sourceEntry = await buildSkillSourceEntry(skill);
if (!sourceEntry) return markdown; if (!sourceEntry) return markdown;
const parsed = parseFrontmatterMarkdown(markdown); const parsed = parseFrontmatterMarkdown(markdown);
const metadata = isPlainRecord(parsed.frontmatter.metadata) const metadata = isPlainRecord(parsed.frontmatter.metadata)
@ -1635,7 +1641,7 @@ export function companyPortabilityService(db: Db) {
for (const skill of companySkillRows) { for (const skill of companySkillRows) {
if (shouldReferenceSkillOnExport(skill, Boolean(input.expandReferencedSkills))) { if (shouldReferenceSkillOnExport(skill, Boolean(input.expandReferencedSkills))) {
files[`skills/${skill.slug}/SKILL.md`] = buildReferencedSkillMarkdown(skill); files[`skills/${skill.slug}/SKILL.md`] = await buildReferencedSkillMarkdown(skill);
continue; continue;
} }
@ -1644,7 +1650,7 @@ export function companyPortabilityService(db: Db) {
if (!fileDetail) continue; if (!fileDetail) continue;
const filePath = `skills/${skill.slug}/${inventoryEntry.path}`; const filePath = `skills/${skill.slug}/${inventoryEntry.path}`;
files[filePath] = inventoryEntry.path === "SKILL.md" files[filePath] = inventoryEntry.path === "SKILL.md"
? withSkillSourceMetadata(skill, fileDetail.content) ? await withSkillSourceMetadata(skill, fileDetail.content)
: fileDetail.content; : fileDetail.content;
} }
} }

View file

@ -70,7 +70,16 @@ function isPlainRecord(value: unknown): value is Record<string, unknown> {
} }
function normalizePortablePath(input: string) { function normalizePortablePath(input: string) {
return input.replace(/\\/g, "/").replace(/^\.\/+/, "").replace(/^\/+/, ""); const parts: string[] = [];
for (const segment of input.replace(/\\/g, "/").replace(/^\.\/+/, "").replace(/^\/+/, "").split("/")) {
if (!segment || segment === ".") continue;
if (segment === "..") {
if (parts.length > 0) parts.pop();
continue;
}
parts.push(segment);
}
return parts.join("/");
} }
function normalizePackageFileMap(files: Record<string, string>) { function normalizePackageFileMap(files: Record<string, string>) {

View file

@ -1190,14 +1190,16 @@ function AgentSkillsTab({
useEffect(() => { useEffect(() => {
if (!skillSnapshot) return; if (!skillSnapshot) return;
if (syncSkills.isPending) return; if (syncSkills.isPending) return;
if (arraysEqual(skillDraft, lastSavedSkills)) return; if (arraysEqual(skillDraft, lastSavedSkillsRef.current)) return;
const timeout = window.setTimeout(() => { const timeout = window.setTimeout(() => {
syncSkills.mutate(skillDraft); if (!arraysEqual(skillDraft, lastSavedSkillsRef.current)) {
syncSkills.mutate(skillDraft);
}
}, 250); }, 250);
return () => window.clearTimeout(timeout); return () => window.clearTimeout(timeout);
}, [lastSavedSkills, skillDraft, skillSnapshot, syncSkills.isPending, syncSkills.mutate]); }, [skillDraft, skillSnapshot, syncSkills.isPending, syncSkills.mutate]);
const companySkillBySlug = useMemo( const companySkillBySlug = useMemo(
() => new Map((companySkills ?? []).map((skill) => [skill.slug, skill])), () => new Map((companySkills ?? []).map((skill) => [skill.slug, skill])),