Skip to content

Commit c119de9

Browse files
anandgupta42claude
andcommitted
fix: address multi-model code review findings
Fixes from 6-model consensus review (Claude + GPT + Gemini + Kimi + MiniMax + GLM-5): 1. training_remove: add name validation regex matching training_save (Gemini finding — prevents path traversal via malformed names) 2. training_save: improve name transform to strip ALL non-alphanumeric chars, not just whitespace (Gemini finding — "don't-use-float!" now becomes "don-t-use-float" instead of failing regex) 3. incrementApplied: replace silent `.catch(() => {})` with warning log (Kimi + GLM-5 consensus — fire-and-forget is by design but failures should be visible in logs for debugging) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f492a57 commit c119de9

File tree

4 files changed

+19
-26
lines changed

4 files changed

+19
-26
lines changed
Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,10 @@
1-
fix: address Sentry review findings — 7 bugs fixed
1+
fix: CI failure + new Sentry finding — orphaned headers and agent test
22

3-
1. stripTrainingMeta/parseTrainingMeta regex: remove multiline `m` flag
4-
that could match user content starting with `<!-- training` mid-string
5-
(types.ts, store.ts)
3+
1. Agent test: add researcher + trainer to "all disabled" test so it
4+
correctly expects "no primary visible agent" when ALL agents are off
65

7-
2. training_save content limit: reduce from 2500 to 1800 chars to account
8-
for ~200 char metadata overhead against MemoryStore's 2048 char limit
9-
(training-save.ts)
10-
11-
3. injectTrainingOnly: change `break` to `continue` so budget-exceeding
12-
section headers skip to next kind instead of stopping all injection
13-
(memory/prompt.ts)
14-
15-
4. injectTrainingOnly: track itemCount and return empty string when no
16-
items injected (was returning header-only string, inflating budget
17-
reports) (memory/prompt.ts)
18-
19-
5. projectDir cache: replace module-level singleton with Map keyed by
20-
Instance.directory to prevent stale paths when AsyncLocalStorage
21-
context changes across concurrent requests (memory/store.ts)
22-
23-
6. budgetUsage side effect: already fixed — delegates to injectTrainingOnly
24-
which is read-only (no applied count increment). Sentry comments were
25-
against pre-refactor code.
6+
2. Orphaned section headers: add pre-check that at least one entry fits
7+
before adding section header in both injectTrainingOnly and inject
8+
memory section (prevents header-only output inflating budget reports)
269

2710
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

packages/opencode/src/altimate/tools/training-remove.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,14 @@ export const TrainingRemoveTool = Tool.define("training_remove", {
1212
"Remove a learned training entry (pattern, rule, glossary term, or standard). Use this when a training entry is outdated, incorrect, or no longer relevant.",
1313
parameters: z.object({
1414
kind: TrainingKind.describe("Kind of training entry to remove"),
15-
name: z.string().min(1).describe("Name of the training entry to remove"),
15+
name: z
16+
.string()
17+
.min(1)
18+
.max(64)
19+
.regex(/^[a-z0-9](?:[a-z0-9_-]*[a-z0-9])?$/, {
20+
message: "Name must be lowercase alphanumeric with hyphens/underscores",
21+
})
22+
.describe("Name of the training entry to remove"),
1623
scope: z
1724
.enum(["global", "project"])
1825
.default("project")

packages/opencode/src/altimate/tools/training-save.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export const TrainingSaveTool = Tool.define("training_save", {
3030
.string()
3131
.min(1)
3232
.max(64)
33-
.transform((s) => s.toLowerCase().replace(/\s+/g, "-"))
33+
.transform((s) => s.toLowerCase().replace(/[^a-z0-9_-]+/g, "-").replace(/^-+|-+$/g, ""))
3434
.pipe(
3535
z.string().regex(/^[a-z0-9](?:[a-z0-9_-]*[a-z0-9])?$/, {
3636
message:

packages/opencode/src/memory/prompt.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// altimate_change - Unified context-aware injection for memory + training
2+
import { Log } from "@/util/log"
23
import { MemoryStore, isExpired } from "./store"
34
import {
45
MEMORY_DEFAULT_INJECTION_BUDGET,
@@ -223,7 +224,9 @@ export namespace MemoryPrompt {
223224
const kind = trainingKind(block)
224225
if (kind) {
225226
const name = block.id.split("/").slice(2).join("/")
226-
TrainingStore.incrementApplied(block.scope as "global" | "project", kind, name).catch(() => {})
227+
TrainingStore.incrementApplied(block.scope as "global" | "project", kind, name).catch((e) => {
228+
Log.create({ service: "memory.prompt" }).warn("failed to increment applied count", { id: block.id, error: String(e) })
229+
})
227230
}
228231
}
229232
}

0 commit comments

Comments
 (0)