Skip to content

fix(memory): leave updated_at alone when a shared memory patch changes nothing - #313

Merged
nathanschmid08 merged 2 commits into
Open-Glassbeetle:mainfrom
nova-loop:fix/shared-memory-noop-patch
Nov 11, 2026
Merged

nathanschmid08 merged 2 commits into
Open-Glassbeetle:mainfrom
nova-loop:fix/shared-memory-noop-patch

Conversation

@nova-loop

Copy link
Copy Markdown
Contributor

What changed

PATCH /memories/:memoryId no longer advances updated_at when the values sent match what is stored. SharedMemoriesService.update() now reads the row first and builds its SET clauses with applySharedMemoryUpdates. That helper was already in shared-memory.mapper.ts (same shape as applyAgentMemoryUpdates) but nothing called it. When nothing differs it returns the stored row untouched.

  • Tags are compared after normalizeTagsOnWrite, so {"tags":["DUP"]} against a stored ["dup"] is no change, and so is {"tags":null} when no tags are stored.
  • A missing memory is now reported before anything is written, as the other services do, instead of being inferred from changes === 0 after the UPDATE.
  • The doc comment on update() now describes the read-then-compare behaviour. The old one talked about avoiding a prior read.

Why

Closes #309

I took option 2 from the issue. The helper already existed, so this is the smaller diff here, and the two memory services now update the same way. The read-then-write race the old comment mentioned can't happen with synchronous better-sqlite3, as the issue explains.

How it was tested

  • New unit cases in shared-memories.service.spec.ts under update: identical content keeps updatedAt; ["DUP"," spaced ","dup"] against stored ["dup","spaced"] keeps updatedAt; tags: null with no stored tags keeps updatedAt; one changed field among identical ones still advances it. I ran the first three against the old update() and all three failed.
  • New e2e case in test/shared-memories.e2e-spec.ts: a PATCH with the stored content and differently-cased tags returns the original updatedAt.
  • In apps/api: npm test (1893 passed), npm run test:e2e (822 passed), npm run lint, and npm run build all pass. I did not run the root workspace build.
  • prettier --check still flags test/shared-memories.e2e-spec.ts, but only for lines that were already there on main (The format check has never run in CI, and 92 committed files no longer pass it #246). The lines I added are formatted.

Anything left out, or worth a second opinion

#311 also touches SharedMemoriesService (it changes the not-found error to MEMORY_NOT_FOUND). Whichever PR lands second will need a small rebase. The 404 here uses the same NotFoundException as findOne, so adopting #311's code is a one-line change.

Checklist

  • npm run lint -w @glassbeetle/api passes (API changes)
  • npm test passes (API workspace)
  • npm run build passes (API workspace)
  • Tests cover the change: failing-first unit tests plus one e2e case
  • Docs updated if behaviour or a convention changed. Only the service doc comment changed; apps/web/README.md already describes this behaviour.
  • The linked issue's acceptance criteria are met

AI assistance

This change was prepared with AI assistance. An AI coding assistant wrote the code and tests from the issue. I reviewed the diff and ran the checks above before opening the PR.

nova-loop and others added 2 commits November 11, 2026 09:56
…p-patch

# Conflicts:
#	apps/api/src/modules/memory/shared-memories/shared-memories.service.ts
@nathanschmid08
nathanschmid08 merged commit 6687c9d into Open-Glassbeetle:main Nov 11, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A PATCH that changes nothing still bumps updated_at on a shared memory, alone among eight resources

2 participants