From 24a1c544f6270b1d8cddff7c3c38cb6cbabb2692 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 9 Mar 2026 19:56:27 +0000 Subject: [PATCH] fix: prevent cross-project knowledge entries from leaking into AGENTS.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-project entries from unrelated projects were appearing in a project's AGENTS.md. Three interrelated causes fixed: 1. Curator saw all cross-project entries and created project-scoped duplicates. Fix: curator and consolidation now use forProject(path, false) to only see project-specific entries. 2. ltm.create() defaulted crossProject to true, so entries imported from AGENTS.md leaked into other projects. Fix: default changed to false; importFromFile now explicitly sets crossProject: false. 3. Consolidation entry count included cross-project entries from all repos, triggering unnecessary consolidation. Fix: use forProject(path, false) for the count. Additional hardening: - Dedup guard in ltm.create() now also checks cross-project entries by title, preventing duplicate creation even if curator tries. - forProject(path, false) no longer includes project_id IS NULL entries (global entries don't belong to any specific project). - Config crossProject default changed from true to false. forSession() is unaffected — it has its own SQL queries with relevance gating for cross-project entries. --- AGENTS.md | 13 +++--- src/agents-file.ts | 2 + src/config.ts | 2 +- src/curator.ts | 4 +- src/index.ts | 2 +- src/ltm.ts | 20 +++++++++- test/agents-file.test.ts | 86 ++++++++++++++++++++++++++++++++++++++++ test/ltm.test.ts | 75 +++++++++++++++++++++++++++++++++++ 8 files changed, 190 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 01bb783..e7366d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,7 +4,7 @@ ### Architecture -* **buildSection() renders AGENTS.md directly without formatKnowledge**: AGENTS.md export/import: buildSection() iterates DB entries grouped by category, emitting \`\\` markers, serialized via remark. splitFile() scans ALL\_START\_MARKERS (current + historical) — self-healing: N duplicate sections collapse to 1 on next export. Import dedup handled by curator LLM at startup when file changed. Missing marker = hand-written; duplicate UUID = first wins. ltm.create() has title-based dedup guard (case-insensitive, skipped for explicit IDs from cross-machine import). Marker text says 'maintained by the coding agent' (not 'auto-maintained') so LLM agents include it in commits. +* **buildSection() renders AGENTS.md directly without formatKnowledge**: AGENTS.md export/import: buildSection() iterates DB entries grouped by category, emitting \`\\` markers via remark. splitFile() scans ALL\_START\_MARKERS (current + historical) — self-healing: N duplicate sections collapse to 1 on next export. Import dedup handled by curator LLM at startup when file changed. Missing marker = hand-written; duplicate UUID = first wins. ltm.create() has title-based dedup guard (case-insensitive, skipped for explicit IDs from cross-machine import). * **Knowledge entry distribution across projects — worktree sessions create separate project IDs**: Knowledge entries are scoped by project\_id from ensureProject(projectPath). OpenCode worktree sessions (paths like ~/.local/share/opencode/worktree/\/\/) each get their own project\_id. A single repo can have multiple project\_ids: one for the real path, separate ones per worktree session. Project-specific entries (cross\_project=0) are invisible across different project\_ids. Cross-project entries (cross\_project=1) are shared globally. @@ -16,7 +16,7 @@ * **Lore temporal pruning runs after distillation and curation on session.idle**: In src/index.ts, session.idle awaits backgroundDistill and backgroundCurate sequentially before running temporal.prune(). Ordering is critical: pruning must not delete unprocessed messages. Pruning defaults: 120-day retention, 1GB max storage (in .lore.json under pruning.retention and pruning.maxStorage). These generous defaults were chosen because the system was new — earlier proposals of 7d/200MB were based on insufficient data. -* **LTM injection pipeline: system transform → forSession → formatKnowledge → gradient deduction**: LTM is injected via experimental.chat.system.transform hook. Flow: getLtmBudget() computes ceiling as (contextLimit - outputReserved - overhead) \* ltmFraction (default 10%, configurable 2-30%). forSession() loads project-specific entries unconditionally + cross-project entries scored by term overlap, greedy-packs into budget. formatKnowledge() renders as markdown. setLtmTokens() records consumption so gradient deducts it. Key: LTM goes into output.system (system prompt), not the message array — invisible to tryFit(), counts against overhead budget. +* **LTM injection pipeline: system transform → forSession → formatKnowledge → gradient deduction**: LTM injected via experimental.chat.system.transform hook. getLtmBudget() computes ceiling as (contextLimit - outputReserved - overhead) \* ltmFraction (default 10%, configurable 2-30%). forSession() loads project-specific entries unconditionally + cross-project entries scored by term overlap, greedy-packs into budget. formatKnowledge() renders as markdown. setLtmTokens() records consumption so gradient deducts it. Key: LTM goes into output.system (system prompt) — invisible to tryFit(), counts against overhead budget. ### Decision @@ -34,17 +34,14 @@ * **Consola prompt cancel returns truthy Symbol, not false**: When a user cancels a \`consola\` / \`@clack/prompts\` confirmation prompt (Ctrl+C), the return value is \`Symbol(clack:cancel)\`, not \`false\`. Since Symbols are truthy in JavaScript, checking \`!confirmed\` will be \`false\` and the code falls through as if the user confirmed. Fix: use \`confirmed !== true\` (strict equality) instead of \`!confirmed\` to correctly handle cancel, false, and any other non-true values. - -* **Craft v2 GitHub App must be installed per-repo**: The Craft v2 release/publish workflows use \`actions/create-github-app-token@v1\` which requires the GitHub App to be installed on the specific repository. If the app is configured for "Only select repositories", adding a new repo to the Craft pipeline requires manually adding it at GitHub Settings → Installations → \[App] → Configure. The \`APP\_ID\` variable and \`APP\_PRIVATE\_KEY\` secret are set in the \`production\` environment, not at repo level. Symptom: 404 on \`GET /repos/{owner}/{repo}/installation\`. - -* **Lore auto-recovery can infinite-loop without re-entrancy guard**: Three bugs in v0.5.2 caused excessive background LLM requests: (1) Auto-recovery infinite loop — session.error overflow handler injected recovery prompt via session.prompt, which could overflow again → another session.error → loop of 2+ LLM calls/cycle. Fix: recoveringSessions Set as re-entrancy guard. (2) Curator ran every idle — \`onIdle || afterTurns\` short-circuited because onIdle=true. Fix: change \`||\` to \`&&\`. Lesson: boolean flag gating numeric threshold needs AND not OR. (3) shouldSkip() fell back to session.list() on every unknown session (short IDs fail session.get). Fix: remove list fallback, cache in activeSessions after first check. +* **Lore auto-recovery can infinite-loop without re-entrancy guard**: Three v0.5.2 bugs causing excessive background LLM requests: (1) Auto-recovery loop — session.error handler injected recovery prompt → could overflow again → loop. Fix: recoveringSessions Set as re-entrancy guard. (2) Curator ran every idle — \`onIdle || afterTurns\` short-circuited (onIdle=true). Fix: \`||\` → \`&&\`. Lesson: boolean flag gating numeric threshold needs AND not OR. (3) shouldSkip() fell back to session.list() on unknown sessions. Fix: remove list fallback, cache in activeSessions. * **Returning bare promises loses async function from error stack traces**: When an \`async\` function returns another promise without \`await\`, the calling function disappears from error stack traces if the inner promise rejects. A function that drops \`async\` and does \`return someAsyncCall()\` loses its frame entirely. Fix: keep the function \`async\` and use \`return await someAsyncCall()\`. This matters for debugging — the intermediate function name in the stack trace helps locate which code path triggered the failure. ESLint rule \`no-return-await\` is outdated; modern engines optimize \`return await\` in async functions. -* **sgdisk reserves 33 sectors for backup GPT, shrinking partition vs original layout**: When recreating a GPT partition entry with \`sgdisk\`, it sets \`LastUsableLBA\` conservatively — 33 sectors short of disk end to reserve space for the backup GPT table. If the original partition extended to the very last sector (common for factory-formatted exFAT SD cards), the recreated partition will be 33 sectors too small. Windows strictly validates that the exFAT VolumeLength in the VBR matches the GPT partition size and refuses to mount on mismatch ("drive not formatted" error). Fix: patch the exFAT VBR's VolumeLength to match the GPT partition size (PartitionLastLBA - PartitionFirstLBA + 1), then recalculate the exFAT boot region checksum (sector 11). Do NOT extend LastUsableLBA to the disk's last sector — that's where the backup GPT header lives, and Windows will reject the GPT as corrupt if usable range overlaps it. +* **sgdisk reserves 33 sectors for backup GPT, shrinking partition vs original layout**: When recreating a GPT partition with \`sgdisk\`, it sets LastUsableLBA 33 sectors short of disk end for backup GPT. If the original partition extended to the last sector (common for factory-formatted exFAT SD cards), the recreated partition is too small. Windows validates exFAT VolumeLength matches GPT partition size — mismatch causes 'drive not formatted' error. Fix: patch the exFAT VBR's VolumeLength to match GPT partition size (LastLBA - FirstLBA + 1), then recalculate boot region checksum (sector 11). Do NOT extend LastUsableLBA past backup GPT header location. * **Test DB isolation via LORE\_DB\_PATH and Bun test preload**: Lore test suite uses isolated temp DB via test/setup.ts preload (bunfig.toml). Preload sets LORE\_DB\_PATH to mkdtempSync path before any imports of src/db.ts; afterAll cleans up. src/db.ts checks LORE\_DB\_PATH first. agents-file.test.ts needs beforeEach cleanup for intra-file isolation and TEST\_UUIDS cleanup in afterAll (shared with ltm.test.ts). Individual test files don't need close() calls — preload handles DB lifecycle. @@ -58,7 +55,7 @@ * **Lore logging: LORE\_DEBUG gating for info/warn, always-on for errors**: src/log.ts provides three levels: log.info() and log.warn() are suppressed unless LORE\_DEBUG=1 or LORE\_DEBUG=true; log.error() always emits. All write to stderr with \[lore] prefix. This exists because OpenCode TUI renders all stderr as red error text — routine status messages (distillation counts, pruning stats, consolidation) were alarming users. Rule: use log.info() for successful operations and status, log.warn() for non-actionable oddities (e.g. dropping trailing messages), log.error() only in catch blocks for real failures. Never use console.error directly in plugin source files. -* **Lore release process: craft + issue-label publish**: Release flow: (1) Trigger release.yml via workflow\_dispatch with version='auto' — uses getsentry/craft to determine version from commits and create a GitHub issue titled 'publish: BYK/opencode-lore@X.Y.Z'. (2) Label that issue 'accepted' — triggers publish.yml which runs craft publish with npm OIDC trusted publishing, then closes the issue. Do NOT create a release/X.Y.Z branch or bump package.json manually — craft handles versioning with 'auto'. The repo uses a GitHub App token (APP\_ID + APP\_PRIVATE\_KEY) for checkout in both workflows. +* **Lore release process: craft + issue-label publish**: Lore/Craft release pipeline and gotchas: (1) Trigger release.yml via workflow\_dispatch with version='auto' — craft determines version and creates GitHub issue. Label 'accepted' → publish.yml runs craft publish with npm OIDC. Don't create release branches or bump package.json manually. (2) GitHub App must be installed per-repo ('Only select repositories' → add at Settings → Installations). APP\_ID/APP\_PRIVATE\_KEY in \`production\` environment. Symptom: 404 on GET /repos/.../installation. (3) npm OIDC only works for publish — \`npm info\` needs NPM\_TOKEN for private packages (public works without auth). * **PR workflow for opencode-lore: branch → PR → auto-merge**: All changes (including minor fixes and test-only changes) must go through a branch + PR + auto-merge, never pushed directly to main. Workflow: (1) git checkout -b \/\, (2) commit, (3) git push -u origin HEAD, (4) gh pr create --title "..." --body "..." --base main, (5) gh pr merge --auto --squash \. Branch name conventions follow merged PR history: fix/\, feat/\, chore/\. Auto-merge with squash is required (merge commits disallowed). Never push directly to main even for trivial changes. diff --git a/src/agents-file.ts b/src/agents-file.ts index 6050e6b..75ff455 100644 --- a/src/agents-file.ts +++ b/src/agents-file.ts @@ -378,6 +378,7 @@ export function importFromFile(input: { title: entry.title, content: entry.content, scope: "project", + crossProject: false, id: entry.id, }); } @@ -395,6 +396,7 @@ export function importFromFile(input: { title: entry.title, content: entry.content, scope: "project", + crossProject: false, }); } } diff --git a/src/config.ts b/src/config.ts index 55808c2..cc019eb 100644 --- a/src/config.ts +++ b/src/config.ts @@ -50,7 +50,7 @@ export const LoreConfig = z.object({ maxStorage: z.number().min(50).default(1024), }) .default({ retention: 120, maxStorage: 1024 }), - crossProject: z.boolean().default(true), + crossProject: z.boolean().default(false), agentsFile: z .object({ /** Set to false to disable all AGENTS.md export/import behaviour. */ diff --git a/src/curator.ts b/src/curator.ts index eff9589..9ca4603 100644 --- a/src/curator.ts +++ b/src/curator.ts @@ -82,7 +82,7 @@ export async function run(input: { if (recent.length < 3) return { created: 0, updated: 0, deleted: 0 }; const text = recent.map((m) => `[${m.role}] ${m.content}`).join("\n\n"); - const existing = ltm.forProject(input.projectPath, cfg.crossProject); + const existing = ltm.forProject(input.projectPath, false); const existingForPrompt = existing.map((e) => ({ id: e.id, category: e.category, @@ -189,7 +189,7 @@ export async function consolidate(input: { const cfg = config(); if (!cfg.curator.enabled) return { updated: 0, deleted: 0 }; - const entries = ltm.forProject(input.projectPath, cfg.crossProject); + const entries = ltm.forProject(input.projectPath, false); if (entries.length <= cfg.curator.maxEntries) return { updated: 0, deleted: 0 }; const entriesForPrompt = entries.map((e) => ({ diff --git a/src/index.ts b/src/index.ts index 11df8c1..5a8cbbe 100644 --- a/src/index.ts +++ b/src/index.ts @@ -359,7 +359,7 @@ export const LorePlugin: Plugin = async (ctx) => { // Runs after normal curation so newly created entries are counted. // Only triggers when truly over the limit to avoid redundant LLM calls. if (cfg.knowledge.enabled) try { - const allEntries = ltm.forProject(projectPath); + const allEntries = ltm.forProject(projectPath, false); if (allEntries.length > cfg.curator.maxEntries) { log.info( `entry count ${allEntries.length} exceeds maxEntries ${cfg.curator.maxEntries} — running consolidation`, diff --git a/src/ltm.ts b/src/ltm.ts index cf635cf..5f46287 100644 --- a/src/ltm.ts +++ b/src/ltm.ts @@ -40,9 +40,12 @@ export function create(input: { // Dedup guard: if an entry with the same project_id + title already exists, // update its content instead of inserting a duplicate. This prevents the // curator from creating multiple entries for the same concept across sessions. + // Also checks cross-project entries to prevent the curator from creating + // project-scoped duplicates of globally-shared knowledge. // Note: when an explicit id is provided (cross-machine import), skip dedup — // the caller (importFromFile) already handles duplicate detection by UUID. if (!input.id) { + // First check same project_id const existing = ( pid !== null ? db() @@ -61,6 +64,19 @@ export function create(input: { update(existing.id, { content: input.content }); return existing.id; } + + // Also check cross-project entries — prevents creating project-scoped + // duplicates of entries that already exist as cross-project knowledge. + const crossExisting = db() + .query( + "SELECT id FROM knowledge WHERE cross_project = 1 AND LOWER(title) = LOWER(?) AND confidence > 0 LIMIT 1", + ) + .get(input.title) as { id: string } | null; + + if (crossExisting) { + update(crossExisting.id, { content: input.content }); + return crossExisting.id; + } } const id = input.id ?? uuidv7(); @@ -77,7 +93,7 @@ export function create(input: { input.title, input.content, input.session ?? null, - (input.crossProject ?? true) ? 1 : 0, + (input.crossProject ?? false) ? 1 : 0, now, now, ); @@ -128,7 +144,7 @@ export function forProject( return db() .query( `SELECT * FROM knowledge - WHERE (project_id = ? OR project_id IS NULL) + WHERE project_id = ? AND confidence > 0.2 ORDER BY confidence DESC, updated_at DESC`, ) diff --git a/test/agents-file.test.ts b/test/agents-file.test.ts index 2b06f38..be53162 100644 --- a/test/agents-file.test.ts +++ b/test/agents-file.test.ts @@ -755,6 +755,92 @@ describe("round-trip stability", () => { }); }); +// --------------------------------------------------------------------------- +// Cross-project isolation +// --------------------------------------------------------------------------- + +const OTHER_PROJECT = "/test/agents-file/other-project"; + +describe("cross-project isolation", () => { + test("importFromFile creates entries with cross_project = 0", () => { + const remoteId = "019505a1-7c00-7000-8000-aabbccddeeff"; + const section = loreSectionWithEntries([ + { id: remoteId, category: "decision", title: "Auth strategy", content: "OAuth2 with PKCE" }, + ]); + writeFile(section); + + importFromFile({ projectPath: PROJECT, filePath: AGENTS_FILE }); + + const entry = ltm.get(remoteId); + expect(entry).not.toBeNull(); + expect(entry!.cross_project).toBe(0); + }); + + test("hand-written entries imported from AGENTS.md are project-scoped", () => { + writeFile(`${LORE_SECTION_START}\n\n## Long-term Knowledge\n\n### Pattern\n\n* **Hand-written pattern**: Using middleware\n\n${LORE_SECTION_END}\n`); + + importFromFile({ projectPath: PROJECT, filePath: AGENTS_FILE }); + + const entries = ltm.forProject(PROJECT, false); + const match = entries.find((e) => e.title === "Hand-written pattern"); + expect(match).toBeDefined(); + expect(match!.cross_project).toBe(0); + }); + + test("cross-project entries from another project do not appear in exportToFile", () => { + // Create a cross-project entry scoped to a different project + ltm.create({ + category: "gotcha", + title: "Unrelated gotcha from other project", + content: "This should not leak into PROJECT's AGENTS.md", + scope: "global", + crossProject: true, + }); + + // Create a project-specific entry for PROJECT + ltm.create({ + projectPath: PROJECT, + category: "decision", + title: "Project-specific decision", + content: "This belongs to this project", + scope: "project", + crossProject: false, + }); + + exportToFile({ projectPath: PROJECT, filePath: AGENTS_FILE }); + + const content = readFile(); + expect(content).toContain("Project-specific decision"); + expect(content).not.toContain("Unrelated gotcha from other project"); + }); + + test("cross-project entries from another project do not inflate forProject(path, false) count", () => { + // Create cross-project entries in "other" project + ltm.create({ + category: "pattern", + title: "Other project pattern", + content: "Cross-project from elsewhere", + scope: "global", + crossProject: true, + }); + + // Create one entry for PROJECT + ltm.create({ + projectPath: PROJECT, + category: "decision", + title: "Only project entry", + content: "Project-scoped", + scope: "project", + crossProject: false, + }); + + const projectOnly = ltm.forProject(PROJECT, false); + const projectOnlyTitles = projectOnly.map((e) => e.title); + expect(projectOnlyTitles).toContain("Only project entry"); + expect(projectOnlyTitles).not.toContain("Other project pattern"); + }); +}); + // --------------------------------------------------------------------------- // Multi-section deduplication (self-healing) // --------------------------------------------------------------------------- diff --git a/test/ltm.test.ts b/test/ltm.test.ts index b02f739..0483c0b 100644 --- a/test/ltm.test.ts +++ b/test/ltm.test.ts @@ -130,6 +130,81 @@ describe("ltm", () => { }); }); +// --------------------------------------------------------------------------- +// crossProject default and dedup guard +// --------------------------------------------------------------------------- + +describe("ltm — crossProject defaults and dedup", () => { + const PROJ = "/test/ltm/crossproject"; + + beforeEach(() => { + const pid = ensureProject(PROJ); + db().query("DELETE FROM knowledge WHERE project_id = ?").run(pid); + // Clean up cross-project entries created by these tests + db() + .query( + "DELETE FROM knowledge WHERE cross_project = 1 AND title LIKE 'Cross-project dedup%'", + ) + .run(); + }); + + test("create() defaults crossProject to false", () => { + const id = ltm.create({ + projectPath: PROJ, + category: "pattern", + title: "Default cross-project test", + content: "Should default to project-scoped", + scope: "project", + }); + const entry = ltm.get(id); + expect(entry).not.toBeNull(); + expect(entry!.cross_project).toBe(0); + }); + + test("create() with explicit crossProject: true sets cross_project = 1", () => { + const id = ltm.create({ + category: "preference", + title: "Explicit cross-project test", + content: "Explicitly shared globally", + scope: "global", + crossProject: true, + }); + const entry = ltm.get(id); + expect(entry).not.toBeNull(); + expect(entry!.cross_project).toBe(1); + }); + + test("dedup guard catches title match against cross-project entry", () => { + // Create a cross-project entry + const crossId = ltm.create({ + category: "gotcha", + title: "Cross-project dedup test entry", + content: "Original cross-project content", + scope: "global", + crossProject: true, + }); + + // Attempt to create a project-scoped entry with the same title — should + // update the cross-project entry instead of creating a duplicate. + const returnedId = ltm.create({ + projectPath: PROJ, + category: "gotcha", + title: "Cross-project dedup test entry", + content: "Updated content from project scope", + scope: "project", + }); + + expect(returnedId).toBe(crossId); + const entry = ltm.get(crossId); + expect(entry!.content).toBe("Updated content from project scope"); + + // No duplicate should exist + const all = ltm.forProject(PROJ, true); + const matching = all.filter((e) => e.title === "Cross-project dedup test entry"); + expect(matching).toHaveLength(1); + }); +}); + describe("ltm — UUIDv7 IDs", () => { const PROJ = "/test/ltm/uuidv7";