Skip to content

Commit a10bf9e

Browse files
committed
fix: resolve feedback do CodeRabbit (#564)
- save() com guarda de concorrencia, escrita atomica (tmp+rename) e emissao de erro - _ensureLoaded() para lazy-load e prevenir overwrite de estado persistido - Filtro em similaridade bruta (similarity) em vez de score composto no getRelevantDecisions - _escapeMarkdown() para prevenir injecao de markdown/prompt no contexto injetado - Recomendacao neutra em patterns sem outcomes resolvidos - _recomputeRelatedPatterns() atualiza patterns quando outcomes mudam - updateOutcome rejeita PENDING via allowedOutcomes (sem duplicar validacao) - _applyTimeDecay com Math.max no resultado final - Operador ?? em vez de || para defaults (convencao do projeto) - Testes expandidos: 49 casos cobrindo novos comportamentos
1 parent 5c72b29 commit a10bf9e

3 files changed

Lines changed: 292 additions & 82 deletions

File tree

.aiox-core/core/memory/decision-memory.js

Lines changed: 133 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ const STOP_WORDS = new Set([
9393
'than', 'too', 'very', 'just', 'because', 'que', 'para',
9494
'com', 'por', 'uma', 'como', 'mais', 'dos', 'das', 'nos',
9595
]);
96+
9697
const CATEGORY_KEYWORDS = {
9798
[DecisionCategory.ARCHITECTURE]: [
9899
'architecture', 'design', 'pattern', 'module', 'refactor',
@@ -111,7 +112,7 @@ const CATEGORY_KEYWORDS = {
111112
'restart', 'rollback', 'backup', 'restore',
112113
],
113114
[DecisionCategory.WORKFLOW]: [
114-
'workflow', 'pipeline', 'ci',
115+
'workflow', 'pipeline', 'ci', 'automate', 'process',
115116
'merge', 'branch', 'review', 'approve',
116117
],
117118
[DecisionCategory.TESTING]: [
@@ -136,11 +137,12 @@ class DecisionMemory extends EventEmitter {
136137
*/
137138
constructor(options = {}) {
138139
super();
139-
this.projectRoot = options.projectRoot || process.cwd();
140+
this.projectRoot = options.projectRoot ?? process.cwd();
140141
this.config = { ...CONFIG, ...options.config };
141142
this.decisions = [];
142143
this.patterns = [];
143144
this._loaded = false;
145+
this._saving = false;
144146
}
145147

146148
/**
@@ -156,8 +158,8 @@ class DecisionMemory extends EventEmitter {
156158
const data = JSON.parse(raw);
157159

158160
if (data.schemaVersion === this.config.schemaVersion) {
159-
this.decisions = data.decisions || [];
160-
this.patterns = data.patterns || [];
161+
this.decisions = data.decisions ?? [];
162+
this.patterns = data.patterns ?? [];
161163
}
162164
}
163165
} catch {
@@ -170,32 +172,53 @@ class DecisionMemory extends EventEmitter {
170172
}
171173

172174
/**
173-
* Save decisions to disk
175+
* Ensure persisted state has been hydrated before operating.
176+
* Lazy-loads on first access to prevent empty-buffer overwrites.
177+
* @returns {Promise<void>}
178+
* @private
179+
*/
180+
async _ensureLoaded() {
181+
if (!this._loaded) {
182+
await this.load();
183+
}
184+
}
185+
186+
/**
187+
* Save decisions to disk.
188+
* Caps in-memory decisions at maxDecisions, guards against concurrent writes,
189+
* and uses atomic write (tmp + rename) to prevent corruption.
174190
* @returns {Promise<void>}
175191
*/
176192
async save() {
193+
if (this._saving) return;
194+
this._saving = true;
195+
177196
const filePath = path.resolve(this.projectRoot, this.config.decisionsJsonPath);
178197
const dir = path.dirname(filePath);
179198

180-
if (!fs.existsSync(dir)) {
181-
fs.mkdirSync(dir, { recursive: true });
182-
}
199+
try {
200+
if (!fs.existsSync(dir)) {
201+
fs.mkdirSync(dir, { recursive: true });
202+
}
183203

184-
this.decisions = this.decisions.slice(-this.config.maxDecisions);
204+
this.decisions = this.decisions.slice(-this.config.maxDecisions);
185205

186-
const data = {
187-
schemaVersion: this.config.schemaVersion,
188-
version: this.config.version,
189-
updatedAt: new Date().toISOString(),
190-
stats: this.getStats(),
191-
decisions: this.decisions,
192-
patterns: this.patterns,
193-
};
206+
const data = {
207+
schemaVersion: this.config.schemaVersion,
208+
version: this.config.version,
209+
updatedAt: new Date().toISOString(),
210+
stats: this.getStats(),
211+
decisions: this.decisions,
212+
patterns: this.patterns,
213+
};
194214

195-
try {
196-
fs.writeFileSync(filePath, JSON.stringify(data, null, 2), 'utf-8');
197-
} catch {
198-
return;
215+
const tmpPath = `${filePath}.tmp`;
216+
fs.writeFileSync(tmpPath, JSON.stringify(data, null, 2), 'utf-8');
217+
fs.renameSync(tmpPath, filePath);
218+
} catch (err) {
219+
this.emit('save:error', { error: err.message, path: filePath });
220+
} finally {
221+
this._saving = false;
199222
}
200223
}
201224

@@ -227,7 +250,7 @@ class DecisionMemory extends EventEmitter {
227250
description,
228251
rationale,
229252
alternatives,
230-
category: category || this._detectCategory(description),
253+
category: category ?? this._detectCategory(description),
231254
taskContext,
232255
agentId,
233256
outcome: Outcome.PENDING,
@@ -256,12 +279,9 @@ class DecisionMemory extends EventEmitter {
256279
const decision = this.decisions.find(d => d.id === decisionId);
257280
if (!decision) return null;
258281

259-
if (!Object.values(Outcome).includes(outcome)) {
260-
throw new Error(`Invalid outcome: ${outcome}. Use: ${Object.values(Outcome).join(', ')}`);
261-
}
262-
263-
if (outcome === Outcome.PENDING) {
264-
throw new Error('Cannot set outcome back to PENDING');
282+
const allowedOutcomes = [Outcome.SUCCESS, Outcome.PARTIAL, Outcome.FAILURE];
283+
if (!allowedOutcomes.includes(outcome)) {
284+
throw new Error(`Invalid outcome: ${outcome}. Use: ${allowedOutcomes.join(', ')}`);
265285
}
266286

267287
decision.outcome = outcome;
@@ -275,6 +295,9 @@ class DecisionMemory extends EventEmitter {
275295
decision.confidence = Math.max(this.config.minConfidence, decision.confidence - 0.3);
276296
}
277297

298+
// Recompute patterns related to this decision when outcomes change
299+
this._recomputeRelatedPatterns(decision);
300+
278301
this.emit(Events.OUTCOME_UPDATED, decision);
279302
return decision;
280303
}
@@ -289,7 +312,7 @@ class DecisionMemory extends EventEmitter {
289312
* @returns {Object[]} Relevant decisions sorted by relevance
290313
*/
291314
getRelevantDecisions(taskDescription, options = {}) {
292-
const limit = options.limit || this.config.maxInjectedDecisions;
315+
const limit = options.limit ?? this.config.maxInjectedDecisions;
293316
const taskKeywords = this._extractKeywords(taskDescription);
294317

295318
let candidates = this.decisions.filter(d => d.outcome !== Outcome.PENDING);
@@ -302,7 +325,8 @@ class DecisionMemory extends EventEmitter {
302325
candidates = candidates.filter(d => d.outcome === Outcome.SUCCESS);
303326
}
304327

305-
// Score by keyword similarity + confidence with time decay
328+
// Score by keyword similarity + confidence with time decay.
329+
// Gate on raw similarity first (config threshold), then rank by composite score.
306330
const scored = candidates.map(d => {
307331
const similarity = this._keywordSimilarity(taskKeywords, d.keywords);
308332
const decayed = this._applyTimeDecay(d.confidence, d.createdAt);
@@ -311,12 +335,13 @@ class DecisionMemory extends EventEmitter {
311335

312336
return {
313337
decision: d,
338+
similarity,
314339
score: (similarity * 0.6) + (decayed * 0.25) + (outcomeBonus * 0.15),
315340
};
316341
});
317342

318343
return scored
319-
.filter(s => s.score >= this.config.similarityThreshold)
344+
.filter(s => s.similarity >= this.config.similarityThreshold)
320345
.sort((a, b) => b.score - a.score)
321346
.slice(0, limit)
322347
.map(s => ({
@@ -326,7 +351,8 @@ class DecisionMemory extends EventEmitter {
326351
}
327352

328353
/**
329-
* Inject relevant decisions as context for a task (AC7)
354+
* Inject relevant decisions as context for a task (AC7).
355+
* Escapes persisted fields to prevent markdown/prompt injection.
330356
* @param {string} taskDescription - Task description
331357
* @returns {string} Formatted context block
332358
*/
@@ -336,17 +362,21 @@ class DecisionMemory extends EventEmitter {
336362
if (relevant.length === 0) return '';
337363

338364
const lines = [
339-
'## 📋 Relevant Past Decisions',
365+
'## Relevant Past Decisions',
340366
'',
341367
];
342368

343369
for (const d of relevant) {
344-
const outcomeIcon = d.outcome === Outcome.SUCCESS ? '✅' :
345-
d.outcome === Outcome.FAILURE ? '❌' : '⚠️';
370+
const outcomeIcon = d.outcome === Outcome.SUCCESS ? '[OK]' :
371+
d.outcome === Outcome.FAILURE ? '[FAIL]' : '[WARN]';
372+
373+
const safeDesc = this._escapeMarkdown(d.description);
374+
const safeRationale = d.rationale ? this._escapeMarkdown(d.rationale) : '';
375+
const safeNotes = d.outcomeNotes ? this._escapeMarkdown(d.outcomeNotes) : '';
346376

347-
lines.push(`### ${outcomeIcon} ${d.description}`);
348-
if (d.rationale) lines.push(`**Rationale:** ${d.rationale}`);
349-
if (d.outcomeNotes) lines.push(`**Outcome:** ${d.outcomeNotes}`);
377+
lines.push(`### ${outcomeIcon} ${safeDesc}`);
378+
if (safeRationale) lines.push(`**Rationale:** ${safeRationale}`);
379+
if (safeNotes) lines.push(`**Outcome:** ${safeNotes}`);
350380
lines.push(`**Category:** ${d.category} | **Confidence:** ${Math.round(this._applyTimeDecay(d.confidence, d.createdAt) * 100)}%`);
351381
lines.push('');
352382
}
@@ -373,12 +403,12 @@ class DecisionMemory extends EventEmitter {
373403
const byCategory = {};
374404

375405
for (const d of this.decisions) {
376-
byOutcome[d.outcome] = (byOutcome[d.outcome] || 0) + 1;
377-
byCategory[d.category] = (byCategory[d.category] || 0) + 1;
406+
byOutcome[d.outcome] = (byOutcome[d.outcome] ?? 0) + 1;
407+
byCategory[d.category] = (byCategory[d.category] ?? 0) + 1;
378408
}
379409

380410
const successRate = total > 0
381-
? (byOutcome[Outcome.SUCCESS] || 0) / Math.max(1, total - (byOutcome[Outcome.PENDING] || 0))
411+
? (byOutcome[Outcome.SUCCESS] ?? 0) / Math.max(1, total - (byOutcome[Outcome.PENDING] ?? 0))
382412
: 0;
383413

384414
return {
@@ -398,7 +428,7 @@ class DecisionMemory extends EventEmitter {
398428
* @returns {Object[]} Recent decisions
399429
*/
400430
listDecisions(options = {}) {
401-
const limit = options.limit || 20;
431+
const limit = options.limit ?? 20;
402432
let results = [...this.decisions];
403433

404434
if (options.category) {
@@ -452,7 +482,7 @@ class DecisionMemory extends EventEmitter {
452482
}
453483

454484
/**
455-
* Calculate keyword similarity between two keyword sets
485+
* Calculate keyword similarity between two keyword sets (Jaccard index)
456486
* @param {string[]} keywords1
457487
* @param {string[]} keywords2
458488
* @returns {number} Similarity score 0-1
@@ -470,7 +500,9 @@ class DecisionMemory extends EventEmitter {
470500
}
471501

472502
/**
473-
* Apply time-based confidence decay
503+
* Apply time-based confidence decay.
504+
* Returns Math.max(minConfidence, confidence * decayFactor) so the
505+
* final decayed value never drops below the configured floor.
474506
* @param {number} confidence - Original confidence
475507
* @param {string} createdAt - ISO date string
476508
* @returns {number} Decayed confidence
@@ -479,16 +511,30 @@ class DecisionMemory extends EventEmitter {
479511
_applyTimeDecay(confidence, createdAt) {
480512
const ageMs = Date.now() - new Date(createdAt).getTime();
481513
const ageDays = ageMs / (1000 * 60 * 60 * 24);
482-
const decayFactor = Math.max(
483-
this.config.minConfidence,
484-
1 - (ageDays / this.config.confidenceDecayDays) * 0.5,
485-
);
514+
const decayFactor = 1 - (ageDays / this.config.confidenceDecayDays) * 0.5;
515+
const decayedConfidence = confidence * decayFactor;
486516

487-
return Math.max(this.config.minConfidence, confidence * decayFactor);
517+
return Math.max(this.config.minConfidence, decayedConfidence);
488518
}
489519

490520
/**
491-
* Detect recurring patterns in decisions (AC9)
521+
* Escape markdown control sequences from persisted fields to prevent
522+
* injected headings, code fences, or instruction text in context blocks.
523+
* @param {string} text - Raw text to sanitize
524+
* @returns {string} Sanitized text safe for markdown embedding
525+
* @private
526+
*/
527+
_escapeMarkdown(text) {
528+
return text
529+
.replace(/```/g, '\\`\\`\\`')
530+
.replace(/^(#{1,6})\s/gm, '\\$1 ')
531+
.replace(/^>/gm, '\\>')
532+
.replace(/\n/g, ' ');
533+
}
534+
535+
/**
536+
* Detect recurring patterns in decisions (AC9).
537+
* Uses a neutral recommendation when no resolved outcomes exist yet.
492538
* @param {Object} newDecision - The new decision to check against
493539
* @private
494540
*/
@@ -504,15 +550,22 @@ class DecisionMemory extends EventEmitter {
504550
const successCount = outcomes.filter(o => o === Outcome.SUCCESS).length;
505551
const failureCount = outcomes.filter(o => o === Outcome.FAILURE).length;
506552

553+
let recommendation;
554+
if (outcomes.length === 0) {
555+
recommendation = 'Recurring pattern detected. No outcome data yet to evaluate.';
556+
} else if (successCount > failureCount) {
557+
recommendation = 'This approach has historically worked well. Consider reusing.';
558+
} else {
559+
recommendation = 'This approach has historically underperformed. Consider alternatives.';
560+
}
561+
507562
const pattern = {
508563
id: `pattern-${this.patterns.length + 1}`,
509564
category: newDecision.category,
510565
description: `Recurring ${newDecision.category} decision: "${newDecision.description}"`,
511566
occurrences: similar.length + 1,
512567
successRate: outcomes.length > 0 ? successCount / outcomes.length : 0,
513-
recommendation: successCount > failureCount
514-
? 'This approach has historically worked well. Consider reusing.'
515-
: 'This approach has historically underperformed. Consider alternatives.',
568+
recommendation,
516569
detectedAt: new Date().toISOString(),
517570
relatedDecisionIds: [...similar.map(d => d.id), newDecision.id],
518571
};
@@ -533,6 +586,33 @@ class DecisionMemory extends EventEmitter {
533586
}
534587
}
535588

589+
/**
590+
* Recompute recommendation for patterns related to a decision whose outcome changed.
591+
* Prevents stale neutral/negative recommendations from persisting after outcomes resolve.
592+
* @param {Object} decision - The decision that was updated
593+
* @private
594+
*/
595+
_recomputeRelatedPatterns(decision) {
596+
for (const pattern of this.patterns) {
597+
if (!pattern.relatedDecisionIds.includes(decision.id)) continue;
598+
599+
const related = this.decisions.filter(d => pattern.relatedDecisionIds.includes(d.id));
600+
const outcomes = related.map(d => d.outcome).filter(o => o !== Outcome.PENDING);
601+
const successCount = outcomes.filter(o => o === Outcome.SUCCESS).length;
602+
const failureCount = outcomes.filter(o => o === Outcome.FAILURE).length;
603+
604+
if (outcomes.length === 0) {
605+
pattern.recommendation = 'Recurring pattern detected. No outcome data yet to evaluate.';
606+
} else if (successCount > failureCount) {
607+
pattern.recommendation = 'This approach has historically worked well. Consider reusing.';
608+
} else {
609+
pattern.recommendation = 'This approach has historically underperformed. Consider alternatives.';
610+
}
611+
612+
pattern.successRate = outcomes.length > 0 ? successCount / outcomes.length : 0;
613+
}
614+
}
615+
536616
/**
537617
* Generate unique decision ID
538618
* @returns {string}

0 commit comments

Comments
 (0)