Skip to content

fix: run profile learning from any active instance under a cross-process lock - #315

Open
doublepi123 wants to merge 2 commits into
tickernelz:mainfrom
doublepi123:fix/profile-learning-cross-process
Open

doublepi123 wants to merge 2 commits into
tickernelz:mainfrom
doublepi123:fix/profile-learning-cross-process

Conversation

@doublepi123

@doublepi123 doublepi123 commented Sep 22, 2026 •

Copy link
Copy Markdown

What

Profile learning currently only runs on the instance that owns the shared web server, so the queue stalls whenever that owner stops seeing sessions and is disabled entirely when the web server is off. Any active instance may now learn: performUserProfileLearning takes a cross-process lock before selecting prompts.

Cross-process lock (rewritten from the original FS lock)

  • Ownership lives in a standalone SQLite coordination DB (.profile-learning-coordination.db under the storage path), deliberately separate from the memory database — ordinary memory writes proceed while the lock is held
  • Acquisition is a single INSERT ... ON CONFLICT DO NOTHING; the winner is decided by rows-affected, and a CAS token guards every takeover
  • No TTL takeover: a conflicting row is only replaced via conditional UPDATE ... WHERE owner_token = <old> when the recorded owner is provably dead (pid + boot_id + starttime identity; non-empty starttime verified in tests). A live owner can never be preempted while deployed
  • Release is DELETE ... WHERE owner_token = <token>, so a stale release from a previous owner (even with a reused PID) can never drop someone else's lock
  • Fail-closed: if the coordination state cannot be trusted, the round is skipped rather than run concurrently

Service wiring

  • The in-process guard is set before the first await so same-process re-entry bounces instead of queueing; release happens in a nested finally so the flag stays raised while the release await is in flight, and still resets when release itself rejects

Cold-buffer hardening

  • Cold buffer is read as bytes with a zero-length guard; short writes are detected (write stalled), never silently truncated into multi-byte corruption
  • Fail-closed schema handling: blank or unexpected-schema files refuse to overwrite instead of being treated as empty

Storage errors propagate (no provider fallback masking)

  • Profile parse/merge failures stay outside the provider fallback catch, so native storage errors propagate instead of being retried through the LLM path

Tests

  • Lock coordination (14), real two-process service lock (8) — including a case proving ordinary memoryClient.addMemory/search/list complete while the learning lock is held, cold-buffer cross-process (16), storage-error no-fallback (4)
  • Combined fork deployment QA: 542 pass / 0 fail / 4 skip

Fixes the stalled-queue and disabled-learning failure modes for multi-instance deployments.

…ess lock

Profile learning was gated on `webServer?.isServerOwner()`. Ownership tracks
whichever process bound the web-server port first and is only handed over when
that process becomes unreachable (web-server.ts:290) — it does not track which
instance the user is actually working in. Two consequences:

- With `webServerEnabled: false` there is no server and therefore no owner, so
  `webServer` stays null (index.ts:244) and automatic profile learning never
  ran at all.
- When the owner stays healthy but stops receiving sessions while another
  project is active, the prompt queue stalls indefinitely. Learning is
  scheduled from a `session.idle` event, and the health check only takes over
  ownership when the server is unreachable, so nothing hands the work over.

Any active instance may now learn. Retention cleanup stays owner-only, since it
is storage-wide maintenance with no reason to run once per instance.

Allowing concurrent learners requires real mutual exclusion. `isLearningRunning`
is a module-level boolean and only prevents re-entry within one process, while
prompt selection is a plain SELECT (user-prompt-manager.ts:270) whose batch is
marked only after the LLM responds. Without cross-process exclusion two
instances would analyze the same prompts and the slower writer would clobber the
faster one's profile update — `updateProfile` re-reads the current version at
write time, so its optimistic check does not detect a concurrent update that
landed while the LLM was running.

This adds a dedicated lock keyed to the shared storage path, held across the
whole learning flow. It deliberately does not reuse `.turso-operation.lock`:
that lock makes `assertNoTursoMigrationInProgress` reject ordinary memory
writes. Contention skips the round rather than waiting, because the next idle
event retries.

The lock also respects a write window before reclaiming an unparseable lock
file. `writeFileSync` is not atomic, so a reader can observe a file that was
created but not yet filled in; reclaiming it on sight would hand the lock to a
second process while the first believes it holds it.

Cold-buffer staleness had to be addressed for the same reason. The buffer is
read once per process and `saveColdBuffers` rewrites the whole file from that
in-memory map, so a process holding a stale map would erase entries a peer
persisted. The manager now reloads it when the file's mtime changes, keeping
the requirement inside the manager rather than relying on callers.

Tests run the real plugin in an isolated process (following
compaction-agent-preservation.test.ts) instead of string-slicing the handler out
of the transpiled source, and the lock is covered by two genuinely concurrent
processes synchronised on a start barrier. Both were verified to fail when the
corresponding protection is removed.
@karaaslanz

Copy link
Copy Markdown
Contributor

I traced the new lock's stale-reclaim path because the PR's correctness depends on mutual exclusion across the full LLM/write flow.
One issue looks worth fixing before relying on it: readLiveLock() reclaims any lock older than 30 minutes even when isProcessAlive(state.pid) is true. The test reclaims a lock held past the staleness deadline actually locks this behavior in by writing the live parent test PID and expecting the child to steal it after 31 minutes.
That means a genuinely live profile-learning run that exceeds the TTL loses mutual exclusion and a second process can analyze/write the same batch — exactly the race this PR is meant to prevent.
The comment says the TTL is for the dead-holder/PID-reuse case, but age alone cannot distinguish a reused PID from the original live holder. I'd keep a confirmed-live holder authoritative, or add a stronger owner identity/heartbeat before allowing stale reclamation, then add a regression that an old-but-still-live holder is not stolen.

- Replace the FS lock with a standalone SQLite coordination DB: CAS token
  ownership, no TTL takeover, dead-owner reclaim via pid+boot_id+starttime
- Move the in-process guard before the first await; release in a nested
  finally so the flag stays raised while release is in flight
- Read the cold buffer as bytes with a zero-length guard and fail closed on
  blank/unexpected-schema files instead of treating them as empty
- Keep profile parse/merge failures outside the provider fallback catch so
  storage errors propagate instead of retrying a broken LLM path
- Add cross-process tests: real service lock (8), cold buffer (16), storage
  error (4), lock coordination (14); ordinary memory writes verified to
  proceed while the learning lock is held
- Fix lint: unused import/variable, require() imports -> ESM

Combined fork deployment QA: 542 pass / 0 fail / 4 skip.
doublepi123 added a commit to doublepi123/opencode-mem that referenced this pull request Sep 30, 2026
Merge fix/profile-learning-cross-process@7e071c5 (hardened) into a1e6a85.

- Keeps the deployed prompt-injection filter (module, config gating,
  authoredParts path) and its 23 tests from main
- Resolves the session.idle conflict in favor of tickernelz#315's structure:
  profile learning runs on every active instance, independent of web-server
  ownership; retention cleanup stays owner-only. Preserves the idle-trigger
  diagnostic log
- Adopts tickernelz#315's real-plugin-subprocess idle tests (profile-learning-idle)
- Brings in the hardened cross-process lock (SQLite coordination DB, CAS
  token ownership, no TTL takeover), byte-exact cold-buffer reads with
  fail-closed schema handling, and storage-error propagation outside the
  provider fallback catch
- Combined fork deployment QA: 542 pass / 0 fail / 4 skip

Tree c52661f verified against the
deployed candidate build.
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.

2 participants