Repository navigation
fix: guard autonomous memory extractor writes - #396
Conversation
|
@davioliveeira is attempting to deploy a commit to the Compozy Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe memory scanner rejects operational-state identifiers. Autonomous writes no longer update existing targets when generated filenames collide without an explicit filename. Controller tests verify no-op results, target preservation, absent replay content, and tiebreaker bypass. ChangesMemory safety corrections
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR filters operational memory content and prevents autonomous slug collisions from overwriting existing memories while preserving explicit user updates. No actionable merge-blocking risk remains beyond normal checks. Sequence Diagram(s)sequenceDiagram
participant Candidate as Memory candidate
participant Controller as Memory controller
participant Target as Existing target
participant Tiebreaker as Tiebreaker
Candidate->>Controller: Submit generated filename
Controller->>Target: Check exact filename collision
alt Autonomous write without explicit target
Controller->>Controller: Create noop decision
Controller-->>Candidate: Return existing target ID without replay
else Explicit target or non-autonomous write
Controller->>Tiebreaker: Continue collision resolution
Controller-->>Candidate: Return update decision with prior content
end
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
| Filename | Overview |
|---|---|
| internal/memory/controller/controller.go | Delegates exact-filename collision handling to the new origin-aware collision decision path. |
| internal/memory/controller/controller_collision.go | Prevents autonomous generated-slug collisions from establishing update identity while retaining explicit-target updates. |
| internal/memory/controller/controller_test.go | Covers generated and explicit collisions for extractor, provider, and dreaming origins; the previously reported parallel-subtest requirement is satisfied. |
| internal/memory/scan/scan.go | Adds deterministic rejection of Memory v2 operational identifiers and policy names. |
| internal/memory/scan/scan_test.go | Exercises qualified tools, unqualified tools, operations, events, and policy identifiers against the new scanner rule. |
| skills/compozy/references/memory.md | Documents that Memory v2 operational identifiers are rejected from durable memory. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Memory candidate] --> B{Operational identifier?}
B -->|Yes| C[Reject without write]
B -->|No| D{Filename collision?}
D -->|No| E[Continue semantic matching]
D -->|Yes| F{Autonomous without explicit target?}
F -->|Yes| G[No-op and preserve existing memory]
F -->|No| H[Update explicitly targeted memory]
Reviews (2): Last reviewed commit: "fix: adjustments" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
pedronauck
left a comment
pedronauck
left a comment
There was a problem hiding this comment.
Changes requested
-
Cover the full canonical Memory v2 operational surface in the scanner. The new closed matcher in internal/memory/scan/scan.go:238 does not match canonical identifiers such as compozy__memory_note, compozy__memory_search, compozy__memory_reindex, memory.write.reverted, or policy_memory_operational_state itself. Those candidates reach Controller.Decide and can be persisted, which contradicts this PR goal. Derive the matcher from the canonical inventory or cover the controlled family, and add isolated table-driven regression cases for every identifier family.
-
Complete the collision-boundary regression coverage. The new controller tests exercise only OriginExtractor with a generated filename. Cover Provider and Dreaming as autonomous origins, and prove that an explicitly supplied target filename still permits the intended direct update. This protects both sides of the new guard.
The reported lack of t.Parallel is not valid; the new subtests already run in parallel.
This branch was successfully deployed
2 participants
Summary
memory_propose, nativecompozy__memory_*tool names, controller event names, and scanner rule IDsRoot cause
The extractor prompt described the correct
WHAT_NOT_TO_SAVEpolicy, but the deterministic scanner did not recognize operational Memory v2 identifiers. Model-generated operational chatter could therefore reach curated memory.Separately, exact filename collisions were treated as update identity before semantic matching. For autonomous candidates, the filename is generated from model-supplied type/entity data, so unrelated facts could overwrite an existing entry with the same slug.
Autonomous generated collisions now produce a deterministic
noopand do not invoke the LLM tiebreaker. This also prevents a tiebreakeraddresult from writing over the colliding filename. Explicitly targeted writes keep the existing update behavior.Fixes #395.
Validation
go test ./internal/memory/scan ./internal/memory/controllergo test ./internal/memory/...go test -race ./internal/memory/scan ./internal/memory/controllermake gate-fullwas also attempted twice. Its Go/module verification completed, butCodegenCheckis blocked in this checkout by the local Bun runtime (1.1.22-canary) differing from the repository's declared[email protected]; OpenAPI generation fails while resolving TypeScript/Redocly dependencies.bun install --frozen-lockfilewas run and the gate retried once, but the environment-level failure remained. No generated or dependency files are included in this PR.Summary by CodeRabbit