Skip to content

feat(opencode): add memory system and GLM thinking corruption fix - #56

Merged
terisuke merged 6 commits into
devfrom
feat/glm-thinking-fix-and-memory-system
Apr 5, 2026
Merged

feat(opencode): add memory system and GLM thinking corruption fix#56
terisuke merged 6 commits into
devfrom
feat/glm-thinking-fix-and-memory-system

Conversation

@terisuke

Copy link
Copy Markdown

Summary

Closes#37
Closes#43
Closes#44
Closes#45
Closes#52

Fact-checking performed

ReferenceStatusFinding
Upstream PR anomalyco#20344 (memory)OPEN, not mergedSchema aligned (project_path, topic, session_id, access_count)
Upstream PR anomalyco#15382 (think tag)CLOSED, rejectedApproach referenced but independently implemented
Upstream Issue anomalyco#16903 (GLM-5)OPEN, known bugWorkaround implemented, not conflicting

Claude Code & claude-code-skills comparison

FeatureClaude CodeOpenCode (this PR)Gap
Memory persistenceFile-based MEMORY.mdSQLite + File dual-layerSuperset
Memory injectionSystem promptSystem prompt (200 line cap)Parity
Auto-extractionManual only5 pattern types auto-detectedUnique
Think tag handlingN/A (Claude native)stripThinkTags utilityMulti-model
Repetition detectionNot documentedStreaming abort at 50+ repeatsUnique
Fact-checking hooks5 hooks in claude-code-skillsNot yet implementedIssue #53

Follow-up Issues created

Codex review findings (all resolved)

  • [P1] MemoryExtractor wired into session processor pipeline
  • [P2] Phantom access_count increment in store.update() fixed
  • [P2] beast.txt memory path inconsistency documented

Test plan

  • bun run --cwd packages/opencode typecheck -- 0 errors
  • 42 unit tests across 6 files -- all pass
    • test/util/format-think.test.ts (9 tests) -- stripThinkTags
    • test/session/repetition.test.ts (8 tests) -- detectRepetition
    • test/memory/file.test.ts (10 tests) -- file ops + path traversal
    • test/memory/store.test.ts (8 tests) -- CRUD + access_count
    • test/memory/extractor.test.ts (6 tests) -- pattern extraction
    • test/memory/abort-leak.test.ts (1 test) -- abort cleanup
  • Local deploy: binary build success, DB migration auto-applied, TUI + API server verified

Generated with Claude Code

CopilotAI review requested due to automatic review settings April 5, 2026 06:08
@github-actions

Copy link
Copy Markdown

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a cross-session “memory” feature (SQLite table + file-based index injection) and mitigations for GLM-5.1 thinking-output corruption by adding think-tag stripping and streaming repetition-loop detection.

Changes:

  • Introduces a new memory SQLite table + MemoryStore/Extractor/File/Injector modules and config flags.
  • Adds GLM-specific system prompt selection and display-layer <think>/<thinking> tag stripping.
  • Implements streaming repetition-loop detection and updates test harness/layers + adds unit tests.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 10 comments.

Show a summary per file
FileDescription
packages/opencode/src/memory/types.tsDefines memory types and shared DTOs.
packages/opencode/src/memory/memory.sql.tsAdds Drizzle schema for memory table + indexes.
packages/opencode/src/memory/store.tsImplements SQLite CRUD + access_count behavior.
packages/opencode/src/memory/file.tsAdds .opencode/memory/ file I/O (index + frontmatter entries) with traversal protection.
packages/opencode/src/memory/injector.tsLoads MEMORY.md and formats injected “Memory” system section.
packages/opencode/src/memory/extractor.tsTracks session patterns and flushes extracted memories to the store.
packages/opencode/src/memory/index.tsRe-exports memory module entry points.
packages/opencode/src/session/prompt.tsInjects memory section into system prompt and uses stripThinkTags() for title cleanup.
packages/opencode/src/session/processor.tsAdds repetition-loop detection + wires MemoryExtractor tracking/cleanup.
packages/opencode/src/session/system.tsSelects new GLM-specific prompt file.
packages/opencode/src/session/prompt/glm.txtAdds GLM-specific system prompt guidance to reduce corruption.
packages/opencode/src/util/format.tsAdds stripThinkTags() helper for <think>/<thinking> parsing.
packages/opencode/src/storage/schema.tsExports MemoryTable from storage schema barrel.
packages/opencode/src/config/config.tsAdds memory.enabled, memory.auto_extract, memory.max_memory_lines config schema.
packages/opencode/migration/20260405053632_add-memory-table/migration.sqlCreates memory table + indexes.
packages/opencode/migration/20260405053632_add-memory-table/snapshot.jsonUpdates migration snapshot to include memory table.
packages/opencode/test/memory/store.test.tsDB-level CRUD tests for memory table operations.
packages/opencode/test/memory/file.test.tsTests file I/O, truncation, listing, and traversal rejection.
packages/opencode/test/memory/extractor.test.tsTests extraction tracking + cleanup behavior.
packages/opencode/test/util/format-think.test.tsTests stripThinkTags() behavior.
packages/opencode/test/session/repetition.test.tsTests repetition-loop detection logic (replicated algorithm).
packages/opencode/test/scenario/harness.tsFixes test harness layering by adding Question/Todo layers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.where(eq(MemoryTable.id, id))
.run(),
)
return toInfo(row)

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MemoryStore.get() increments access_count in the DB but returns the pre-update row, so the returned accessCount is stale (still the old value). This can be confusing for callers that expect the returned object to reflect the read side-effect; consider re-selecting after the update (or manually returning accessCount + 1) so the returned value matches persisted state.

Suggested change
returntoInfo(row)
returntoInfo({
...row,
access_count: (row.access_count??0)+1,
})

Copilot uses AI. Check for mistakes.
Comment on lines +115 to +123
// Fire-and-forget memory extraction helper, gated by config
const memoryExtract = (fn: () => void | Promise<void>) => {
void Config.get()
.then((cfg) => {
if (cfg.memory?.auto_extract === false) return
return fn()
})
.catch((err) => log.warn("memory extractor error", { error: err }))
}

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memoryExtract() gates only on cfg.memory.auto_extract but ignores cfg.memory.enabled. With enabled=false, the extractor will still run and write to SQLite, which contradicts the meaning of the top-level enable/disable flag. Consider short-circuiting when memory.enabled is false (and also ensuring cleanup still runs for any in-memory session state).

Copilot uses AI. Check for mistakes.
Comment on lines +115 to 125
// Fire-and-forget memory extraction helper, gated by config
const memoryExtract = (fn: () => void | Promise<void>) => {
void Config.get()
.then((cfg) => {
if (cfg.memory?.auto_extract === false) return
return fn()
})
.catch((err) => log.warn("memory extractor error", { error: err }))
}

const create = Effect.fn("SessionProcessor.create")(function* (input: Input) {

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memoryExtract() calls Config.get() for every tracked tool/result/error event, which can add a lot of async overhead during high-frequency streaming/tool use. Since auto_extract is effectively static during a session, consider reading config once in create()/process() and capturing a boolean flag (or using the injected config service) instead of reloading it on every event.

Suggested change
// Fire-and-forget memory extraction helper, gated by config
constmemoryExtract=(fn: ()=>void|Promise<void>)=>{
voidConfig.get()
.then((cfg)=>{
if(cfg.memory?.auto_extract===false)return
returnfn()
})
.catch((err)=>log.warn("memory extractor error",{error: err}))
}
constcreate=Effect.fn("SessionProcessor.create")(function*(input: Input){
constcreate=Effect.fn("SessionProcessor.create")(function*(input: Input){
constautoExtractEnabled=(yield*Config.get()).memory?.auto_extract!==false
// Fire-and-forget memory extraction helper, gated by config captured
// once for this processor run to avoid repeated async config lookups.
constmemoryExtract=(fn: ()=>void|Promise<void>)=>{
if(!autoExtractEnabled)return
voidPromise.resolve(fn()).catch((err)=>log.warn("memory extractor error",{error: err}))
}

Copilot uses AI. Check for mistakes.
Comment on lines 170 to +176
case "reasoning-delta":
if (!(value.id in ctx.reasoningMap)) return
ctx.reasoningMap[value.id].text += value.text
if (value.providerMetadata) ctx.reasoningMap[value.id].metadata = value.providerMetadata
if (detectRepetition(ctx.reasoningMap[value.id].text)) {
yield* Effect.fail(new RepetitionError(ctx.reasoningMap[value.id].text.slice(-80)))
}

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detectRepetition() is invoked on every reasoning/text delta once the buffer exceeds 8KB, and its inner loop scans 4..200 pattern lengths each time. On fast streaming models this can become a hot path. Consider throttling checks (e.g., only every N characters / N deltas), or maintaining incremental state so the per-delta cost is bounded.

Copilot uses AI. Check for mistakes.
Comment on lines +1502 to +1509
const [skills, env, instructions, modelMsgs, memory] = yield* Effect.all([
Effect.promise(() => SystemPrompt.skills(agent)),
Effect.promise(() => SystemPrompt.environment(model)),
instruction.system().pipe(Effect.orDie),
Effect.promise(() => MessageV2.toModelMessages(msgs, model)),
Effect.promise(() => MemoryInjector.load()),
])
const system = [...env, ...(skills ? [skills] : []), ...instructions]
const system = [...env, ...(skills ? [skills] : []), ...instructions, ...(memory ? [memory] : [])]

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SessionPrompt injects memory via MemoryInjector.load(), which reads .opencode/memory/MEMORY.md, but the auto-extraction pipeline currently persists entries via MemoryStore (SQLite) only. With no DB→file sync in this PR, auto-extracted memories won't ever show up in the injected prompt. Consider either (a) loading injected memory from MemoryStore, or (b) having the extractor update MEMORY.md / entry files so both layers stay consistent.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +17
import { Bus } from "@/bus"
import { Config } from "@/config/config"
import { Log } from "@/util/log"
import { MemoryStore } from "./store"
import type { Memory } from "./types"
import { Instance } from "@/project/instance"

const log = Log.create({ service: "memory.extractor" })

type SessionState = {
commands: Map<string, number>
errors: string[]
fixes: string[]
configChanges: string[]
lastFlush: number
pending: Memory.Create[]
}

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memory/extractor.ts has unused imports (Bus, Config) and unused SessionState fields (fixes, configChanges). Besides compiler/lint noise, the unused arrays can grow without bound if tracking is called repeatedly. Consider removing the unused imports/fields or wiring them into logic that actually consumes them (e.g., for summarization/dedup).

Copilot uses AI. Check for mistakes.
Comment on lines +113 to +127
state.pending = []
state.lastFlush = Date.now()

for (const entry of batch) {
await MemoryStore.runPromise((svc) => svc.create(entry)).catch((err) => {
log.warn("failed to flush memory entry", { error: err, topic: entry.topic })
})
}
log.info("flushed memory entries", { sessionID, count: batch.length })
}

export async function cleanup(sessionID: string) {
await flush(sessionID).catch((err) => {
log.warn("failed to flush on cleanup", { error: err })
})

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flush() clears state.pending before attempting writes. If MemoryStore.create fails (e.g., transient DB errors), the entry is dropped permanently and cannot be retried. Consider only removing entries from pending after a successful create, or collecting failed entries and re-queueing them for the next flush/cleanup.

Suggested change
state.pending=[]
state.lastFlush=Date.now()
for(constentryofbatch){
awaitMemoryStore.runPromise((svc)=>svc.create(entry)).catch((err)=>{
log.warn("failed to flush memory entry",{error: err,topic: entry.topic})
})
}
log.info("flushed memory entries",{ sessionID,count: batch.length})
}
exportasyncfunctioncleanup(sessionID: string){
awaitflush(sessionID).catch((err)=>{
log.warn("failed to flush on cleanup",{error: err})
})
constfailed: typeofbatch=[]
state.pending=[]
state.lastFlush=Date.now()
for(constentryofbatch){
awaitMemoryStore.runPromise((svc)=>svc.create(entry)).catch((err)=>{
failed.push(entry)
log.warn("failed to flush memory entry",{error: err,topic: entry.topic})
})
}
if(failed.length>0){
state.pending=failed.concat(state.pending)
log.warn("failed to flush some memory entries; queued for retry",{
sessionID,
failed: failed.length,
attempted: batch.length,
})
}
log.info("flushed memory entries",{
sessionID,
count: batch.length-failed.length,
})
}
exportasyncfunctioncleanup(sessionID: string){
awaitflush(sessionID).catch((err)=>{
log.warn("failed to flush on cleanup",{error: err})
})
conststate=sessions.get(sessionID)
if(state&&state.pending.length>0){
log.warn("skipping session cleanup because memory entries are still pending",{
sessionID,
pending: state.pending.length,
})
return
}

Copilot uses AI. Check for mistakes.
Comment threadpackages/opencode/src/memory/file.ts Outdated
Comment on lines +20 to +22
const dir = memoryDir()
const resolved = path.resolve(dir, filename)
if (!resolved.startsWith(dir + path.sep) && resolved !== dir)

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entryPath() allows resolved === dir, which means an empty filename (""), ".", etc. would pass the traversal check and result in attempting to read/write the directory path as a file. Consider rejecting empty filenames and any filename that resolves to the directory itself, and/or enforcing that filename is a basename (no path separators).

Suggested change
constdir=memoryDir()
constresolved=path.resolve(dir,filename)
if(!resolved.startsWith(dir+path.sep)&&resolved!==dir)
if(!filename||filename==="."||filename===".."||path.basename(filename)!==filename)
thrownewError("invalid filename")
constdir=memoryDir()
constresolved=path.resolve(dir,filename)
if(!resolved.startsWith(dir+path.sep))

Copilot uses AI. Check for mistakes.
Comment on lines +37 to +44
if (!fm.topic || !fm.type) return undefined
return {
frontmatter: {
topic: fm.topic,
type: fm.type as Memory.Type,
},
content: match[2].trim(),
}

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseFrontmatter() casts the type string directly to Memory.Type without validating it against the allowed type list. Since these files are user-editable, invalid types can silently propagate through the system. Consider validating fm.type (and fm.topic) and returning undefined (or throwing a clear error) when the value isn't one of Memory.TYPES.

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +27

// detectRepetition is a namespace-internal function in SessionProcessor.
// We replicate the algorithm here to test it in isolation.
// Constants match processor.ts: REPETITION_THRESHOLD=50, REPETITION_WINDOW=8000

const REPETITION_THRESHOLD = 50
const REPETITION_WINDOW = 8000

function detectRepetition(text: string): boolean {
if (text.length < REPETITION_WINDOW) return false
const tail = text.slice(-REPETITION_WINDOW)
for (let len = 4; len <= 200; len++) {
const pattern = tail.slice(-len)
let count = 0
let pos = tail.length - len
while (pos >= 0) {
if (tail.slice(pos, pos + len) === pattern) {
count++
pos -= len
} else break
}
if (count >= REPETITION_THRESHOLD) return true
}
return false
}

CopilotAIApr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test duplicates the detectRepetition algorithm and constants instead of exercising the production implementation. This can easily drift (e.g., threshold/window changes) without failing tests. Consider extracting detectRepetition into a shared util that both processor.ts and the test import, or exporting a small test-only helper/constant from processor.ts so the test stays coupled to the real logic.

Suggested change
// detectRepetition is a namespace-internal function in SessionProcessor.
// We replicate the algorithm here to test it in isolation.
// Constants match processor.ts: REPETITION_THRESHOLD=50, REPETITION_WINDOW=8000
constREPETITION_THRESHOLD=50
constREPETITION_WINDOW=8000
functiondetectRepetition(text: string): boolean{
if(text.length<REPETITION_WINDOW)returnfalse
consttail=text.slice(-REPETITION_WINDOW)
for(letlen=4;len<=200;len++){
constpattern=tail.slice(-len)
letcount=0
letpos=tail.length-len
while(pos>=0){
if(tail.slice(pos,pos+len)===pattern){
count++
pos-=len
}elsebreak
}
if(count>=REPETITION_THRESHOLD)returntrue
}
returnfalse
}
import{
detectRepetition,
REPETITION_THRESHOLD,
REPETITION_WINDOW,
}from"../../src/session/processor"

Copilot uses AI. Check for mistakes.
terisukeand others added 5 commits April 5, 2026 16:03
Implements cross-session persistent memory (Issues #43, #44, #45) and
fixes GLM-5.1 thinking output corruption (Issue #52).
Memory system: SQLite + file-based storage with auto-extraction and
session-start injection, schema aligned with upstream PR anomalyco#20344.
GLM fix: model-specific prompt, think tag parsing utility, and
streaming repetition loop detection to prevent context exhaustion.
Closes#43, Closes#44, Closes#45, Relates #52
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes pre-existing type error where ToolRegistry.layer dependencies
(Question.Service, Todo.Service) were not provided in harness.ts.
Relates #43
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wire MemoryExtractor into session processor pipeline (P1)
- Fix phantom access_count increment in store.update() (P2)
- Document beast.txt memory path inconsistency (P2)
Refs #37, #43, #44
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove unused Bus and Config imports
- Add sessions Map eviction at 100 entries to prevent memory leak
- Add .catch() to maybeFlush to handle unhandled promise rejection
Refs #43, #44, #45
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- get() returns fresh access_count after increment
- memoryExtract respects memory.enabled flag
- Cache config read instead of per-event Config.get()
- Sync extracted memories to MEMORY.md file system
- Retry failed flush entries instead of dropping
- Reject empty filenames in entryPath
Refs #43, #44, #45
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@terisuke
terisukeforce-pushed the feat/glm-thinking-fix-and-memory-system branch from 3991f79 to 79106aaCompareApril 5, 2026 07:08
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants

@terisuke