Skip to content

Commit 5386309

Browse files
fix: close 3 P1s from the fix-delta re-audit (merge gate, v36 seed, Pi ownership)
Re-audit of the audit-fix commits surfaced 3 P1s against the zero-P1 ship bar: P1 #1 — ctx_memory MERGE skipped the workspace category gate. My earlier P0 fix routed update/archive through memoryVisibleToTool but left merge on the weaker memoryBelongsToProject / storedPathBelongsToIdentity (the commit comment claimed parity it didn't have). A primary agent could thus consolidate a foreign member's memory in a non-shared category — the exact invisible-mutation the P0 closed for update/archive. Routed merge through memoryVisibleToTool on BOTH harnesses. +2 plugin regression tests (foreign non-shared merge refused; foreign shared merge allowed). P1 #2 — v36 created session_projects empty, so every session chunk-embedded BEFORE v36 became invisible to the project-scoped backfill/count JOINs. Seed the table in the migration from compartment_chunk_embeddings (the only pre-existing source carrying session_id+harness+project_path — compartments has NO project_path column, so the auditor's proposed SELECT FROM compartments was unrunnable). Seed only unambiguous sessions (single distinct project_path); ambiguous pre-scope-bug sessions are left for the idempotent heal path. Guarded on the source table existing. +1 seed test (single-project seeded per-harness, ambiguous skipped). P1 #3 — Pi never recorded session→project ownership (grep-confirmed zero calls), so Pi-published compartments were invisible to /ctx-embed-history's project scope. Wired recordSessionProjectIdentity into Pi's updateSessionProjectTracking, fired only on the once-per-(session,identity) transition it already detects — ctx.cwd is authoritative in Pi so every observation is host-safe; no per-pass write. Context the auditors lacked (verified via callgraph): the project-WIDE passive sweep is test-only now (its dream-timer driver was removed in 9a91ac7); the only live backfill is /ctx-embed-history, which already self-records ownership. So #2/#3 were latent (half-populated table) rather than a live-path break — fixed anyway so the v36 table ships complete. plugin 2029/0, Pi 461/0, biome clean both. Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent 9134aa5 commit 5386309

6 files changed

Lines changed: 161 additions & 8 deletions

File tree

packages/pi-plugin/src/context-handler.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import {
5252
parseCacheTtl,
5353
type Scheduler,
5454
} from "@magic-context/core/features/magic-context/scheduler";
55+
import { recordSessionProjectIdentity } from "@magic-context/core/features/magic-context/session-project-storage";
5556
import {
5657
adoptFallbackTagMessageId,
5758
type ContextDatabase,
@@ -443,6 +444,7 @@ function isContextHandlerSessionActive(sessionId: string): boolean {
443444
function updateSessionProjectTracking(
444445
sessionId: string,
445446
projectIdentity: string,
447+
db?: ContextDatabase,
446448
): void {
447449
const prev = lastSeenProjectIdentityBySession.get(sessionId);
448450
if (prev && prev !== projectIdentity) {
@@ -451,6 +453,21 @@ function updateSessionProjectTracking(
451453
if (prevSessions?.size === 0) sessionsByProject.delete(prev);
452454
clearPiSystemPromptSession(sessionId);
453455
}
456+
// Persist the session→project ownership binding so the project-scoped
457+
// compartment-chunk backfill (/ctx-embed-history) can attribute this
458+
// session's compartments to the right project. ctx.cwd is the authoritative
459+
// session directory in Pi (no SDK/launch-dir ambiguity), so every observation
460+
// is host-safe. Guarded to the once-per-(session,identity) transition — only
461+
// on first sight or an actual identity change — so steady-state passes carry
462+
// no per-pass DB write. embedSessionCompartmentChunks also self-records, so
463+
// this only widens coverage to passively-published sessions.
464+
if (db && prev !== projectIdentity) {
465+
try {
466+
recordSessionProjectIdentity(db, sessionId, projectIdentity);
467+
} catch {
468+
// best-effort; backfill re-records on demand from the session command
469+
}
470+
}
454471
trackSessionForProject(projectIdentity, sessionId);
455472
lastSeenProjectIdentityBySession.set(sessionId, projectIdentity);
456473
}
@@ -1373,7 +1390,7 @@ export function registerPiContextHandler(
13731390
const schedulerConfig = options.scheduler ?? DEFAULT_SCHEDULER_CONFIG;
13741391
const scheduler = schedulerFor(options);
13751392
const projectIdentity = resolveProjectIdentity(projectDirectory);
1376-
updateSessionProjectTracking(sessionId, projectIdentity);
1393+
updateSessionProjectTracking(sessionId, projectIdentity, options.db);
13771394
logTransformTiming(
13781395
sessionId,
13791396
"findSessionId",

packages/pi-plugin/src/tools/ctx-memory.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -472,10 +472,7 @@ export function createCtxMemoryTool(
472472
// must not reach into ANOTHER project's memories — mirror
473473
// update/archive ownership (parity with OpenCode).
474474
if (!dreamerAllowed) {
475-
const foreign = sourceMemories.find(
476-
(memory) =>
477-
!storedPathBelongsToIdentity(memory.projectPath, projectIdentity),
478-
);
475+
const foreign = sourceMemories.find((memory) => !memoryVisibleToTool(memory));
479476
if (foreign) {
480477
return err(`Error: Memory with ID ${foreign.id} was not found.`);
481478
}

packages/plugin/src/features/magic-context/migrations-v36.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,48 @@ describe("migration v36 — session project ownership", () => {
6363
closeQuietly(db);
6464
}
6565
});
66+
67+
test("v36 seeds ownership for already-chunk-embedded sessions, skipping ambiguous ones", () => {
68+
const db = new Database(":memory:");
69+
try {
70+
// A v35 DB whose chunk-embedding table predates the ownership map.
71+
db.exec(`
72+
CREATE TABLE schema_migrations (version INTEGER PRIMARY KEY, description TEXT NOT NULL, applied_at INTEGER NOT NULL);
73+
INSERT INTO schema_migrations (version, description, applied_at) VALUES (35, 'pre-v36 fixture', 1);
74+
CREATE TABLE compartment_chunk_embeddings (
75+
compartment_id INTEGER NOT NULL,
76+
session_id TEXT NOT NULL,
77+
harness TEXT NOT NULL DEFAULT 'opencode',
78+
project_path TEXT NOT NULL,
79+
window_index INTEGER NOT NULL,
80+
chunk_hash TEXT NOT NULL,
81+
model_id TEXT NOT NULL,
82+
vector BLOB NOT NULL
83+
);
84+
-- clean single-project session (two windows, same project): seed it
85+
INSERT INTO compartment_chunk_embeddings VALUES (1,'sesA','opencode','git:alpha',0,'h1','m',x'00');
86+
INSERT INTO compartment_chunk_embeddings VALUES (1,'sesA','opencode','git:alpha',1,'h2','m',x'00');
87+
-- same session id under a DIFFERENT harness: independent ownership row
88+
INSERT INTO compartment_chunk_embeddings VALUES (2,'sesA','pi','git:beta',0,'h3','m',x'00');
89+
-- ambiguous session (pre-scope bug split across two projects): skip
90+
INSERT INTO compartment_chunk_embeddings VALUES (3,'sesB','opencode','git:gamma',0,'h4','m',x'00');
91+
INSERT INTO compartment_chunk_embeddings VALUES (3,'sesB','opencode','git:delta',0,'h5','m',x'00');
92+
`);
93+
94+
runMigrations(db);
95+
96+
const rows = db
97+
.prepare(
98+
"SELECT session_id, harness, project_path FROM session_projects ORDER BY session_id, harness",
99+
)
100+
.all();
101+
// sesA/opencode→alpha and sesA/pi→beta seeded; sesB skipped (ambiguous).
102+
expect(rows).toEqual([
103+
{ session_id: "sesA", harness: "opencode", project_path: "git:alpha" },
104+
{ session_id: "sesA", harness: "pi", project_path: "git:beta" },
105+
]);
106+
} finally {
107+
closeQuietly(db);
108+
}
109+
});
66110
});

packages/plugin/src/features/magic-context/migrations.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,6 +1407,32 @@ const MIGRATIONS: Migration[] = [
14071407
CREATE INDEX IF NOT EXISTS idx_session_projects_project
14081408
ON session_projects(project_path);
14091409
`);
1410+
// Seed ownership for sessions that were ALREADY chunk-embedded before
1411+
// this table existed, so their compartments stay visible to the
1412+
// project-scoped backfill/count JOINs (a fresh table would otherwise
1413+
// hide every pre-v36 embedded session until re-observed). The only
1414+
// trustworthy pre-existing source is compartment_chunk_embeddings
1415+
// itself, which already carries (session_id, harness, project_path).
1416+
// NOTE: compartments has no project_path column — do not read from it.
1417+
// Seed ONLY unambiguous sessions (a single distinct project_path);
1418+
// skip any session whose chunks are split across projects (the
1419+
// pre-scope bug) so the heal path, not a coin-flip, decides its owner.
1420+
// Guarded on the source table existing — it is created at v33, so any
1421+
// real upgrade has it, but a partial/older fixture might not.
1422+
const hasChunkTable = db
1423+
.prepare(
1424+
"SELECT 1 FROM sqlite_master WHERE type='table' AND name='compartment_chunk_embeddings'",
1425+
)
1426+
.get();
1427+
if (hasChunkTable) {
1428+
db.exec(`
1429+
INSERT OR IGNORE INTO session_projects (session_id, harness, project_path, updated_at)
1430+
SELECT session_id, harness, MIN(project_path), 0
1431+
FROM compartment_chunk_embeddings
1432+
GROUP BY session_id, harness
1433+
HAVING COUNT(DISTINCT project_path) = 1;
1434+
`);
1435+
}
14101436
},
14111437
},
14121438
];

packages/plugin/src/tools/ctx-memory/tools.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,77 @@ describe("createCtxMemoryTools", () => {
511511
expect(getMemoryById(db, own.id)?.status).toBe("archived");
512512
});
513513

514+
it("REFUSES a PRIMARY merge that pulls in a foreign memory in a NON-shared category", async () => {
515+
// merge MUST gate on the same own/foreign-by-category visibility as
516+
// update/archive: a primary agent cannot consolidate a foreign member's
517+
// memory it can't even see in its rendered context.
518+
db.exec(`
519+
INSERT INTO workspaces (id, name, created_at, updated_at, share_categories)
520+
VALUES (1, 'ws', 1, 1, '["CONSTRAINTS"]');
521+
INSERT INTO workspace_members (workspace_id, project_path, display_name, display_path, added_at)
522+
VALUES (1, '/repo/project', 'Own', '/repo/project', 1),
523+
(1, '/repo/foreign', 'Foreign', '/repo/foreign', 1);
524+
`);
525+
const own = insertMemory(db, {
526+
projectPath: "/repo/project",
527+
category: "ARCHITECTURE",
528+
content: "Own architecture detail A.",
529+
});
530+
const foreignHidden = insertMemory(db, {
531+
projectPath: "/repo/foreign",
532+
category: "ARCHITECTURE",
533+
content: "Foreign architecture detail not shared with this project.",
534+
});
535+
536+
const result = await tools.ctx_memory.execute(
537+
{
538+
action: "merge",
539+
ids: [own.id, foreignHidden.id],
540+
content: "Merged architecture detail.",
541+
category: "ARCHITECTURE",
542+
},
543+
toolContext(),
544+
);
545+
546+
expect(result).toContain(`Memory with ID ${foreignHidden.id} was not found`);
547+
expect(getMemoryById(db, own.id)?.status).toBe("active");
548+
expect(getMemoryById(db, foreignHidden.id)?.status).toBe("active");
549+
});
550+
551+
it("allows a PRIMARY merge of a foreign memory in a SHARED category", async () => {
552+
db.exec(`
553+
INSERT INTO workspaces (id, name, created_at, updated_at, share_categories)
554+
VALUES (1, 'ws', 1, 1, '["CONSTRAINTS"]');
555+
INSERT INTO workspace_members (workspace_id, project_path, display_name, display_path, added_at)
556+
VALUES (1, '/repo/project', 'Own', '/repo/project', 1),
557+
(1, '/repo/foreign', 'Foreign', '/repo/foreign', 1);
558+
`);
559+
const own = insertMemory(db, {
560+
projectPath: "/repo/project",
561+
category: "CONSTRAINTS",
562+
content: "Own constraint A.",
563+
});
564+
const foreignShared = insertMemory(db, {
565+
projectPath: "/repo/foreign",
566+
category: "CONSTRAINTS",
567+
content: "Foreign constraint shared with this project.",
568+
});
569+
570+
const result = await tools.ctx_memory.execute(
571+
{
572+
action: "merge",
573+
ids: [own.id, foreignShared.id],
574+
content: "Merged shared constraint.",
575+
category: "CONSTRAINTS",
576+
},
577+
toolContext(),
578+
);
579+
580+
expect(result).not.toContain("was not found");
581+
expect(getMemoryById(db, own.id)?.status).toBe("archived");
582+
expect(getMemoryById(db, foreignShared.id)?.status).toBe("archived");
583+
});
584+
514585
describe("#given update action", () => {
515586
it("updates a foreign workspace memory with duplicate checks and mutations under the target identity", async () => {
516587
db.exec(`

packages/plugin/src/tools/ctx-memory/tools.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -493,9 +493,7 @@ function createCtxMemoryTool(deps: CtxMemoryToolDeps): ToolDefinition {
493493
// its own resolved project. The dreamer keeps the cross-identity
494494
// path (see the "merging across identities" test).
495495
if (toolContext.agent !== DREAMER_AGENT) {
496-
const foreign = sourceMemories.find(
497-
(memory) => !memoryBelongsToProject(memory, projectPath),
498-
);
496+
const foreign = sourceMemories.find((memory) => !memoryVisibleToTool(memory));
499497
if (foreign) {
500498
return `Error: Memory with ID ${foreign.id} was not found.`;
501499
}

0 commit comments

Comments
 (0)