Context
Same session as the upsertNode root-cause ticket (filed as sibling). This ticket closes the symptom-layer hazard that the upsertNode fix enables full elimination of; filed independently so each can land and be reviewed in isolation.
The Problem
ai/mcp/server/memory-core/services/PermissionService.mjs:65-68 fabricates a minimal AGENT:<x> stub if the target isn't in the in-memory cache when grantPermission is called:
if (!db.nodes.get(to)) {
GraphService.upsertNode({ id: to, type: 'AGENT', name: to.replace('AGENT:', ''), properties: {} });
}
Two independent hazards compound here:
- In-memory check only (addressed by sibling upsertNode ticket at the upsertNode substrate layer): if
to is a seeded identity ('AGENT:*', '@neo-opus-4-7', etc.) that exists in SQLite but isn't yet cache-loaded, the condition returns falsy and the stub is upserted. Upstream fix prevents metadata destruction but doesn't fix #2.
- Stub shape is architecturally wrong: even with correct cache behavior, the intent is wrong. Identity creation is a privileged operation via the seed script; it should NOT be an implicit side-effect of a permission grant. The lazy-stub pattern creates identity nodes with
type: 'AGENT' and stripped metadata — neither AgentIdentity nor BroadcastSentinel. Any downstream consumer reading type-specific metadata gets garbage.
Empirical evidence: AGENT:* observed with type 'AGENT' (lazy-stub shape) instead of 'BroadcastSentinel' (seed shape from #10174). Attribution: grantPermission({to: 'AGENT:*', ...}) called before AGENT:* was in cache → stub upserted → SQLite overwritten.
The Architectural Reality
ai/mcp/server/memory-core/services/PermissionService.mjs:65-68 — lazy-stub path
- Precedent for strict check:
GraphService.linkNodes:179-185 queries SQLite directly and CULLS the edge if the target doesn't exist, logging a warning
- Identity creation substrate:
ai/scripts/seedAgentIdentities.mjs — the ONE place that should upsert AgentIdentity / BroadcastSentinel nodes
- Existing specs (
MailboxService.spec.mjs, PermissionService.spec.mjs post-#10229) already pre-seed their test fixtures; this ticket's stricter behavior aligns with the established spec pattern
The Fix
Replace the lazy-stub path with SQLite direct existence check, throwing a clear error for non-existent targets:
async grantPermission({ to, scope }) {
const owner = RequestContextService.getAgentIdentityNodeId();
if (!owner) throw new Error("Cannot grant permission: no agent identity context bound.");
if (!to) throw new Error("Missing 'to' parameter.");
if (!scope) throw new Error("Missing 'scope' parameter.");
if (!this.validScopes.includes(scope)) {
throw new Error(`Invalid scope. Must be one of: ${this.validScopes.join(', ')}`);
}
const verifyStmt = GraphService.db.storage.db.prepare('SELECT count(*) as count FROM Nodes WHERE id = ?');
if (verifyStmt.get(to).count === 0) {
throw new Error(`Cannot grant ${scope} to ${to}: target does not exist. Identity nodes must be pre-seeded via ai/scripts/seedAgentIdentities.mjs.`);
}
GraphService.linkNodes(to, owner, scope, 1.0);
return { success: true, message: `Granted ${scope} to ${to}` };
}
Acceptance Criteria
Out of Scope
- Changes to revokePermission (edge-only deletion, no upsert hazard)
- Changes to the AGENT:* sentinel semantics
- Role-based addressing (
role:<name>) and human addressing (human:<login>) — these are write-permissive by design; only the AGENT:/@-prefixed identity path has the stub hazard
Avoided Traps
- "Just use linkNodes' cull-if-missing behavior": rejected. Silent culling obscures the "target is mis-typed" class of errors. Throwing surfaces the misuse loudly, aligning with the "surface, don't obscure" discipline from PR #10227.
- "Create full AgentIdentity stub if missing": rejected. Identity metadata (
githubLogin, modelFamily, accountType) is seed-provisioned; permission grant has no knowledge of these. Any stub would be wrong metadata.
- "Wait for sibling upsertNode fix to eliminate the hazard at the substrate layer": rejected in part. The upsertNode fix eliminates the cold-cache corruption of existing nodes, but does NOT fix the architectural misuse of upsertNode-from-grantPermission-with-wrong-type. Both tickets needed.
Related
- Root cause (substrate layer): sibling ticket "upsertNode must lazy-load from SQLite before cache decision"
- Existing correct pattern:
GraphService.linkNodes:179-185 (SQLite FK-style check)
- Seed source of truth:
ai/scripts/seedAgentIdentities.mjs
- Parent epic: #10139 Mailbox A2A primitive
Origin Session ID: 24aa1fa1-9a22-498e-97f2-760c12e5a79d
Handoff Retrieval Hints
query_raw_memories(query="PermissionService grantPermission lazy stub AGENT type")
query_raw_memories(query="AgentIdentity wipe partial state overwrite")
Context
Same session as the upsertNode root-cause ticket (filed as sibling). This ticket closes the symptom-layer hazard that the upsertNode fix enables full elimination of; filed independently so each can land and be reviewed in isolation.
The Problem
ai/mcp/server/memory-core/services/PermissionService.mjs:65-68fabricates a minimalAGENT:<x>stub if the target isn't in the in-memory cache whengrantPermissionis called:if (!db.nodes.get(to)) { // It's technically fine if the target doesn't exist yet, but linkNodes might cull it if not found in SQLite. // Let's lazily ensure the node exists if we're granting permission to it. GraphService.upsertNode({ id: to, type: 'AGENT', name: to.replace('AGENT:', ''), properties: {} }); }Two independent hazards compound here:
tois a seeded identity ('AGENT:*','@neo-opus-4-7', etc.) that exists in SQLite but isn't yet cache-loaded, the condition returns falsy and the stub is upserted. Upstream fix prevents metadata destruction but doesn't fix #2.type: 'AGENT'and stripped metadata — neitherAgentIdentitynorBroadcastSentinel. Any downstream consumer reading type-specific metadata gets garbage.Empirical evidence:
AGENT:*observed with type'AGENT'(lazy-stub shape) instead of'BroadcastSentinel'(seed shape from #10174). Attribution:grantPermission({to: 'AGENT:*', ...})called before AGENT:* was in cache → stub upserted → SQLite overwritten.The Architectural Reality
ai/mcp/server/memory-core/services/PermissionService.mjs:65-68— lazy-stub pathGraphService.linkNodes:179-185queries SQLite directly and CULLS the edge if the target doesn't exist, logging a warningai/scripts/seedAgentIdentities.mjs— the ONE place that should upsertAgentIdentity/BroadcastSentinelnodesMailboxService.spec.mjs,PermissionService.spec.mjspost-#10229) already pre-seed their test fixtures; this ticket's stricter behavior aligns with the established spec patternThe Fix
Replace the lazy-stub path with SQLite direct existence check, throwing a clear error for non-existent targets:
async grantPermission({ to, scope }) { const owner = RequestContextService.getAgentIdentityNodeId(); if (!owner) throw new Error("Cannot grant permission: no agent identity context bound."); if (!to) throw new Error("Missing 'to' parameter."); if (!scope) throw new Error("Missing 'scope' parameter."); if (!this.validScopes.includes(scope)) { throw new Error(`Invalid scope. Must be one of: ${this.validScopes.join(', ')}`); } // Verify target exists in SQLite directly (not in-memory cache, per sibling ticket's // lazy-load root-cause fix). Identity creation is a privileged operation (seedAgentIdentities.mjs); // it should NOT be an implicit side-effect of a permission grant. Stubbing with type 'AGENT' // + stripped metadata destroys seed data and creates type-inconsistent nodes. // Pattern mirrors linkNodes:179-185 (FK-style existence guard). const verifyStmt = GraphService.db.storage.db.prepare('SELECT count(*) as count FROM Nodes WHERE id = ?'); if (verifyStmt.get(to).count === 0) { throw new Error(`Cannot grant ${scope} to ${to}: target does not exist. Identity nodes must be pre-seeded via ai/scripts/seedAgentIdentities.mjs.`); } GraphService.linkNodes(to, owner, scope, 1.0); return { success: true, message: `Granted ${scope} to ${to}` }; }Acceptance Criteria
grantPermission({to: '<seeded-identity>', ...})succeeds AND preserves the existing node's type/properties (no stub overwrite)grantPermission({to: '<non-existent-id>', ...})throws a clear error citing the seed script as the correct pathAGENT:*asBroadcastSentinel, callgrantPermission({to: 'AGENT:*', scope: 'CAN_REPLY_TO'}), assert the node's type remainsBroadcastSentinelpost-grantgrantPermission({to: 'AGENT:phantom', ...})on a fresh DB, assert throws with "must be pre-seeded" messageOut of Scope
role:<name>) and human addressing (human:<login>) — these are write-permissive by design; only the AGENT:/@-prefixed identity path has the stub hazardAvoided Traps
githubLogin,modelFamily,accountType) is seed-provisioned; permission grant has no knowledge of these. Any stub would be wrong metadata.Related
GraphService.linkNodes:179-185(SQLite FK-style check)ai/scripts/seedAgentIdentities.mjsOrigin Session ID:
24aa1fa1-9a22-498e-97f2-760c12e5a79dHandoff Retrieval Hints
query_raw_memories(query="PermissionService grantPermission lazy stub AGENT type")query_raw_memories(query="AgentIdentity wipe partial state overwrite")