feat(memory): add PawWork memory v1 - #520
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThis PR implements PawWork's transparent memory system: a ChangesCore Memory System
Session & API Integration
User Interface
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
packages/opencode/src/session/prompt.ts (1)
1828-1848: ⚡ Quick winAvoid reading
MEMORY.mdon every loop step.This block runs inside the main loop, so Line 1830 can re-read disk multiple times in one turn. Consider loading once per
runLoopcycle and reusing the value unless a write happens in-turn.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/session/prompt.ts` around lines 1828 - 1848, The memory profile is being read from disk on every loop iteration because MemoryService.create(...).read() is called inside the loop where memoryProfile is computed; change this to load the MEMORY.md once per runLoop cycle and reuse it for each iteration unless an in-turn write occurs. Implement a small cache in the runLoop scope (e.g., a local variable like cachedMemoryProfile and cachedMemoryVersion/timestamp) and replace the direct MemoryService.create(...).read() usage in the memoryProfile computation with a lookup from that cache; when any code path performs a write to memory during the runLoop, invalidate/update the cache so subsequent loop iterations read the new value. Ensure you keep the existing error handling (Effect.catch) and the same string formatting around the profile when using the cached value.packages/opencode/src/server/instance/memory.ts (2)
20-20: ⚡ Quick winConsider a more specific schema for MemoryState.
Using
z.any()bypasses validation and type safety. If the actual MemoryState shape is defined elsewhere (e.g., in@/memory/service), reference or reuse that schema here for consistency.🛡️ Improve type safety
If
MemoryService.read()returns a typed object, define its schema:const MemoryState = z.object({ disabled: z.boolean().optional(), status: z.enum(["ok", "safe_mode"]).optional(), // ... other fields from MemoryService.read() }).meta({ ref: "MemoryState" })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/server/instance/memory.ts` at line 20, The MemoryState schema currently uses z.any() which bypasses validation; replace it with a concrete Zod schema that matches the actual shape returned by MemoryService.read() (e.g., include fields like disabled:boolean, status: "ok" | "safe_mode", and any other properties from MemoryService.read()) or import and reuse the existing schema from the memory service module; update the MemoryState declaration (symbol: MemoryState) to use that object/enum schema or the imported schema so type safety and validation are enforced across usages of MemoryState and MemoryService.read().
22-24: ⚖️ Poor tradeoffService instance is recreated on every request.
The
service()helper creates a newMemoryServiceinstance for each route handler call. IfMemoryService.create()is lightweight and stateless, this is fine; otherwise, consider caching the instance per workspace.Currently this matches the stateless pattern used elsewhere in the instance routes (e.g., creating clients/services inline per request). Unless
MemoryServiceinitialization is expensive, the existing approach is acceptable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/server/instance/memory.ts` around lines 22 - 24, service() currently calls MemoryService.create({ workspacePath: Instance.directory }) on every invocation which recreates the service per request; if MemoryService initialization is expensive you should cache it per workspace instead. Modify the helper (service) to use a module-level cache (e.g., a Map keyed by Instance.directory or a single variable if only one workspace) so that service() returns the existing MemoryService instance when present, otherwise calls MemoryService.create(...) and stores it; reference MemoryService.create, service() and Instance.directory when implementing the cache.packages/app/src/components/memory/session-memory-review.tsx (1)
7-10: 💤 Low valueType definition duplicated across files.
The local
MemoryStatetype mirrors the shape returned by the backend. If this type is shared between frontend and backend (or generated from OpenAPI), consider importing from a shared location to ensure consistency.If the SDK client already generates types from the OpenAPI schema, use those instead of a local definition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/components/memory/session-memory-review.tsx` around lines 7 - 10, The local MemoryState type in session-memory-review.tsx duplicates the backend/OpenAPI shape; remove this local definition and import the canonical type instead (e.g., the generated SDK/OpenAPI type or the shared backend types) to keep types consistent; update any references to MemoryState in this file (and export/usage in components) to use the imported symbol so the frontend relies on the single source of truth rather than the duplicated local declaration.packages/opencode/src/tool/memory-search.ts (1)
20-22: ⚡ Quick winConsider adding error context for searchArchive failures.
The
Effect.promisewrapper will propagate any rejection fromsearchArchiveas an Effect failure, but without additional context about what query failed. When debugging tool execution failures, knowing the query parameter can help.🔍 Optional: Add error context
- const result = yield* Effect.promise(() => - MemoryService.create({ workspacePath: ins.directory }).searchArchive(params.query), - ) + const result = yield* Effect.promise(() => + MemoryService.create({ workspacePath: ins.directory }).searchArchive(params.query), + ).pipe( + Effect.mapError((error) => new Error(`Memory search failed for query "${params.query}": ${error}`)) + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/src/tool/memory-search.ts` around lines 20 - 22, The searchArchive call inside Effect.promise can fail without context; update the call that creates result (the Effect.promise wrapping MemoryService.create(...).searchArchive(...)) to catch any rejection and rethrow or reject with an error that includes params.query and ins.directory (or other identifying info) so the Effect failure message contains the query and workspace; use MemoryService.create, searchArchive, params.query and ins.directory as the referenced symbols when adding the catch/augmenting the error before returning from Effect.promise.packages/app/src/components/settings-memory.tsx (1)
33-58: ⚡ Quick winConsider adding operation-specific loading states.
The buttons are disabled only during the initial resource fetch (
state.loading), not during save/reset/toggle/delete operations. This allows rapid repeated clicks and provides no visual feedback during operations.♻️ Add operation loading signal
const language = useLanguage() const sdk = useSDK() const [draft, setDraft] = createSignal("") const [deleteID, setDeleteID] = createSignal("") +const [operationLoading, setOperationLoading] = createSignal(false) const [state, actions] = createResource(async () => { const result = await sdk.client.memory.get() const data = (result.data ?? {}) as MemoryState setDraft(data.content ?? "") return data }) const refresh = () => void actions.refetch() const save = async () => { + setOperationLoading(true) + try { await sdk.client.memory.update({ memoryRawInput: { content: draft() } }) showToast({ variant: "success", title: language.t("settings.memory.saved") }) refresh() + } finally { + setOperationLoading(false) + } } // Apply same pattern to reset, toggle, deleteEntry // Then update button disabled states: -<Button variant="primary" onClick={save} disabled={state.loading}> +<Button variant="primary" onClick={save} disabled={state.loading || operationLoading()}>Also applies to: 104-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/components/settings-memory.tsx` around lines 33 - 58, Add per-operation loading flags and use them to disable buttons and show feedback during SDK calls: introduce boolean state variables like isSaving, isResetting, isToggling, isDeleting and set them true before and false after each async call in the save, reset, toggle, and deleteEntry functions (also handle errors/finally to ensure flags are cleared). Update the UI bindings that currently rely only on state.loading to also disable/reflect loading for the corresponding operations (e.g., disable the Save button when isSaving, Reset when isResetting, toggle control when isToggling, Delete when isDeleting) and keep using draft(), deleteID(), showToast(), refresh() as before. Ensure toggle uses the provided enabled param and that each flag is cleared in a finally block so repeated clicks are prevented and users see operation-specific feedback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app/src/components/memory/session-memory-review.tsx`:
- Around line 32-34: The acceptProposal flow exits silently when the API returns
disabled or status === "safe_mode"; update the post-response branch (the code
that handles result from sdk.client.memory.acceptProposal and casts to
MemoryState) so that when state.disabled || state.status === "safe_mode" it (1)
clears the user's draft input (call the component's draft-clearing handler e.g.,
setProposalText or clearDraft), (2) dismisses/closes the review UI (invoke the
existing dismiss/close handler such as onDismiss or closeReview), and (3)
surfaces feedback to the user (use the app's notification mechanism, e.g.,
showToast/enqueueSnackbar/showNotification) explaining acceptance is blocked;
place these calls immediately inside the early-return branch that currently just
returns.
In `@packages/app/src/components/settings-memory.tsx`:
- Around line 33-37: Wrap each async action handler (save, reset, toggle,
deleteEntry) in a try/catch: perform the SDK call in try, call refresh() and
success showToast on success, and in catch log the error (console.error or
process logger) and show an error showToast with a descriptive message and the
error message; ensure any local state (e.g., loading flags) is cleared in
finally if used. Locate the functions save, reset, toggle, and deleteEntry in
settings-memory.tsx and apply the same pattern to each async SDK invocation so
users receive feedback on failures.
- Around line 22-29: The createResource wrapper around sdk.client.memory.get
lacks error handling; update the async function passed to createResource (the
one that sets state/actions) to wrap the call in try/catch, handle failures from
sdk.client.memory.get, and surface the error via the resource state or a new
signal so the UI can show feedback. Specifically, catch errors, log them, set a
safe fallback MemoryState (e.g., empty content) and setDraft("") (or appropriate
fallback), and return that fallback or rethrow to let the resource populate its
error state so consumers of state/actions can render an error message.
In `@packages/opencode/src/memory/memory.ts`:
- Around line 102-104: The current serialization of project scope metadata puts
input.appliesTo directly into the scope token (in the expression that assigns
scope in memory.ts), which is later parsed by splitting metadata on whitespace
and thus loses spaces in paths; update the serializer to encode or quote
appliesTo (e.g., wrap it in quotes or use JSON.stringify/encodeURIComponent)
when building the scope string (`const scope = ...` that uses input.appliesTo)
and update the corresponding parser that splits metadata (the metadata
split/parse logic that consumes the scope/applies_to token around lines
~145-165) to decode/unquote the value so paths like "C:\Users\Jane Doe\repo"
round-trip intact.
- Around line 112-116: The current parseEntries implementation splits the
archive using /\n(?=### )/, which breaks entries if their body contains markdown
"### " lines; update parseEntries to split or extract whole entries only at
headings that start at the beginning of a line. Replace the simple split with a
multiline-aware approach (e.g. use a regex like /(^### [\s\S]*?)(?=^### |\z)/gm
to match full entry blocks, or use archive.split(/\n(?=^### )/m)) so that only
headings at line-start (not inline/body subheadings) delimit entries; adjust
subsequent code that uses chunks/entries accordingly.
In `@packages/opencode/src/memory/service.ts`:
- Around line 66-73: The read-modify-write sequences (e.g., writeAtomic and
callers like appendAcceptedProposal and the other affected blocks) are
vulnerable to race conditions; serialize mutation operations by introducing an
in-process write queue or mutex: add a single Promise-based queue (or a simple
lock) that all functions performing read-modify-write await before calling
ensure(), reading the file, and invoking writeAtomic, then release the lock when
done; update writeAtomic and every caller listed (the appendAcceptedProposal
code path and the other ranges noted) to acquire the queue/lock before
performing the read-modify-write and to chain the promise so concurrent callers
are executed serially. Ensure the queue/lock is shared at module scope so all
mutations go through it.
In `@packages/opencode/src/tool/memory-search.ts`:
- Around line 1-33: The module is missing the canonical self-reexport so
consumers can import it as a namespace; add the module-as-namespace reexport at
the end of the file by appending the line export * as MemorySearch from
"./memory-search" (so code that expects to import the MemorySearch namespace
alongside the exported MemorySearchTool will work).
In `@packages/opencode/test/memory/pawwork-memory.test.ts`:
- Around line 76-77: Replace manual temporary directory creation using
fs.mkdtemp with the repo tmpdir test fixture: import tmpdir from the fixture
(fixture/fixture.ts) and use "await using (const dir = await tmpdir()) { ... }"
so the directory is auto-cleaned; inside the using block pass dir to
MemoryService.createForTest({ home: dir, workspacePath: "/repo/pawwork" })
(update the three occurrences around the current usage at
MemoryService.createForTest calls). Ensure the test file uses the tmpdir fixture
import and the await using pattern for automatic cleanup and consistent setup.
---
Nitpick comments:
In `@packages/app/src/components/memory/session-memory-review.tsx`:
- Around line 7-10: The local MemoryState type in session-memory-review.tsx
duplicates the backend/OpenAPI shape; remove this local definition and import
the canonical type instead (e.g., the generated SDK/OpenAPI type or the shared
backend types) to keep types consistent; update any references to MemoryState in
this file (and export/usage in components) to use the imported symbol so the
frontend relies on the single source of truth rather than the duplicated local
declaration.
In `@packages/app/src/components/settings-memory.tsx`:
- Around line 33-58: Add per-operation loading flags and use them to disable
buttons and show feedback during SDK calls: introduce boolean state variables
like isSaving, isResetting, isToggling, isDeleting and set them true before and
false after each async call in the save, reset, toggle, and deleteEntry
functions (also handle errors/finally to ensure flags are cleared). Update the
UI bindings that currently rely only on state.loading to also disable/reflect
loading for the corresponding operations (e.g., disable the Save button when
isSaving, Reset when isResetting, toggle control when isToggling, Delete when
isDeleting) and keep using draft(), deleteID(), showToast(), refresh() as
before. Ensure toggle uses the provided enabled param and that each flag is
cleared in a finally block so repeated clicks are prevented and users see
operation-specific feedback.
In `@packages/opencode/src/server/instance/memory.ts`:
- Line 20: The MemoryState schema currently uses z.any() which bypasses
validation; replace it with a concrete Zod schema that matches the actual shape
returned by MemoryService.read() (e.g., include fields like disabled:boolean,
status: "ok" | "safe_mode", and any other properties from MemoryService.read())
or import and reuse the existing schema from the memory service module; update
the MemoryState declaration (symbol: MemoryState) to use that object/enum schema
or the imported schema so type safety and validation are enforced across usages
of MemoryState and MemoryService.read().
- Around line 22-24: service() currently calls MemoryService.create({
workspacePath: Instance.directory }) on every invocation which recreates the
service per request; if MemoryService initialization is expensive you should
cache it per workspace instead. Modify the helper (service) to use a
module-level cache (e.g., a Map keyed by Instance.directory or a single variable
if only one workspace) so that service() returns the existing MemoryService
instance when present, otherwise calls MemoryService.create(...) and stores it;
reference MemoryService.create, service() and Instance.directory when
implementing the cache.
In `@packages/opencode/src/session/prompt.ts`:
- Around line 1828-1848: The memory profile is being read from disk on every
loop iteration because MemoryService.create(...).read() is called inside the
loop where memoryProfile is computed; change this to load the MEMORY.md once per
runLoop cycle and reuse it for each iteration unless an in-turn write occurs.
Implement a small cache in the runLoop scope (e.g., a local variable like
cachedMemoryProfile and cachedMemoryVersion/timestamp) and replace the direct
MemoryService.create(...).read() usage in the memoryProfile computation with a
lookup from that cache; when any code path performs a write to memory during the
runLoop, invalidate/update the cache so subsequent loop iterations read the new
value. Ensure you keep the existing error handling (Effect.catch) and the same
string formatting around the profile when using the cached value.
In `@packages/opencode/src/tool/memory-search.ts`:
- Around line 20-22: The searchArchive call inside Effect.promise can fail
without context; update the call that creates result (the Effect.promise
wrapping MemoryService.create(...).searchArchive(...)) to catch any rejection
and rethrow or reject with an error that includes params.query and ins.directory
(or other identifying info) so the Effect failure message contains the query and
workspace; use MemoryService.create, searchArchive, params.query and
ins.directory as the referenced symbols when adding the catch/augmenting the
error before returning from Effect.promise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 033cad88-f480-4f6a-8444-b963cd9f34ac
⛔ Files ignored due to path filters (2)
packages/sdk/js/src/v2/gen/sdk.gen.tsis excluded by!**/gen/**packages/sdk/js/src/v2/gen/types.gen.tsis excluded by!**/gen/**
📒 Files selected for processing (19)
packages/app/src/components/memory/session-memory-review.tsxpackages/app/src/components/settings-memory.test.tspackages/app/src/components/settings-memory.tsxpackages/app/src/components/settings-page.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/pages/session.tsxpackages/app/src/pages/session/session-main-view.tsxpackages/app/src/shell-frame-contract.test.tspackages/core/src/pawwork-home.tspackages/opencode/src/memory/memory.tspackages/opencode/src/memory/proposal.tspackages/opencode/src/memory/service.tspackages/opencode/src/server/instance/index.tspackages/opencode/src/server/instance/memory.tspackages/opencode/src/session/prompt.tspackages/opencode/src/tool/memory-search.tspackages/opencode/src/tool/registry.tspackages/opencode/test/memory/pawwork-memory.test.ts
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive 'Memory' feature to PawWork, enabling persistent user context across sessions via a structured MEMORY.md file. The implementation includes a MemoryService for file operations, a new settings tab for configuration and raw editing, and a SessionMemoryReview component for saving session-specific insights. Additionally, a memory_search tool allows the AI to retrieve historical context, and a redaction mechanism is included to protect sensitive information. Review feedback identifies potential issues with the regular expressions used for parsing and deleting memory entries, specifically regarding how they handle markdown headings within entry bodies, and provides suggestions for more robust matching.
11 tasks
Astro-Han
added a commit
that referenced
this pull request
May 10, 2026
Summary: - Remove the session-end SessionMemoryReview confirmation UI and its mount path so PawWork memory no longer asks users whether to save a session. - Remove the old memory proposal accept/review API surface, proposal redaction helper, and generated SDK methods that only supported the confirmation flow. - Simplify MEMORY.md handling for v1: Profile remains the only section loaded automatically, Archive becomes freeform markdown that is searched/read only on demand, and Settings keeps view/edit/disable/reset control. - Update the memory prompt so the assistant maintains MEMORY.md silently at reply/task closeout, records only explicit stable facts, avoids inference from user tasks, updates conflicts instead of appending contradictions, and never writes sensitive data. Root cause: - PR #520 implemented memory as a user-confirmed session review panel, but the product direction is silent background memory. That UI surfaced an unnecessary confirmation step and made memory feel like an interruptive modal workflow instead of an assistant-maintained local notebook. Scope boundary: - This is not a new memory search system, structured memory schema, hidden patch protocol, or dedicated memory tool. - Archive is intentionally not injected into every session to avoid context growth; the prompt only tells the assistant to grep/read it when historical context is actually needed. - Profile remains short and startup-loaded. Archive remains local history. Review follow-up handled: - GLM first-pass verified the issue #526 spec coverage and found no P0/P1 blockers. - Opus second-pass identified prompt gaps for first-write onboarding and no extrapolated preferences; both were patched. - GPT-X final review confirmed no over-engineering remnants, no unresolved P1/P2 risks, and complete spec coverage. - CodeRabbit and Gemini feedback was addressed: archive grep output is capped, the explicit remember acknowledgement example is English, deleteEntry no longer stops on arbitrary markdown headings, and prompt examples are fully English. Verification: - bun --cwd packages/opencode test test/memory/pawwork-memory.test.ts - bun --cwd packages/app test:unit -- src/shell-frame-contract.test.ts src/components/settings-memory.test.ts - bun --cwd packages/app test:e2e -- e2e/settings/settings-memory.spec.ts - bun --cwd packages/opencode typecheck - bun --cwd packages/app typecheck - bun --cwd packages/sdk/js typecheck - bun --cwd packages/sdk/js build - git diff --check - GitHub checks passed: CI, CodeQL, desktop smoke, e2e artifacts, commit lint, PR title lint, dependency review, CodeRabbit. - GitHub reviewThreads unresolved count verified as 0 before merge. Closes #526
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements PawWork Memory v1 from #519:
~/.pawwork/memory/MEMORY.mdcontract with## Profileand## ArchiveWhy
PawWork sessions currently forget useful cross-session context. The v1 goal is the smallest transparent memory system: user-visible Markdown, no embeddings, no cloud sync, no silent writes, no dedicated memory search tool, and reversible controls.
Related Issue
Closes #519
Human Review Status
Pending. GLM first-pass and Opus second-pass reviewed the earlier versions; after AstroHan/Opus/GPT/Kimi/GLM arbitration, the dedicated
memory_searchtool and v1-only metadata/UI surface were removed for a smaller Bash-grep design.Review Focus
## Profile/## Archive~/.pawwork/memory/MEMORY.mddirectly and keep injected Archive output capped to 2000 charsRisk Notes
~/.pawwork/memory/MEMORY.md, plus.bak,.broken.bak, and.disabledfiles under the same directory.How To Verify
Screenshots or Recordings
Not captured in this Slock run. Visible UI changes are covered by
packages/app/e2e/settings/settings-memory.spec.ts:sk-*style secrets before landing inMEMORY.mdChecklist
dev, and my PR title and commit messages use Conventional Commits in EnglishRequested labels:
type: feature,area: app,area: opencode,priority: P2.