Skip to content

fix(core): serialize config store mutations#719

Open
jsdavid278-cyber wants to merge 2 commits into
profullstack:masterfrom
jsdavid278-cyber:codex/serialize-config-store-writes
Open

fix(core): serialize config store mutations#719
jsdavid278-cyber wants to merge 2 commits into
profullstack:masterfrom
jsdavid278-cyber:codex/serialize-config-store-writes

Conversation

@jsdavid278-cyber

Copy link
Copy Markdown
Contributor

Summary

  • serialize adapter config read-modify-write mutations so concurrent setup operations do not overwrite each other
  • use unique temporary paths for config writes to avoid concurrent temp-file collisions
  • add a regression test covering concurrent set and delete operations

Root cause

setAdapterConfig() and deleteAdapterConfig() independently read and rewrote the same config file. Concurrent calls could read the same snapshot and the last rename would silently discard another mutation.

Validation

  • corepack pnpm vitest run packages/core/src
  • corepack pnpm --filter @profullstack/sh1pt-core typecheck
  • git diff --check

@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a lost-update race in the config store where concurrent setAdapterConfig / deleteAdapterConfig calls could read the same stale snapshot and the last atomic rename would silently discard other mutations. It introduces a module-level promise chain (configMutationLock) to serialize all write operations and switches to per-call unique temp filenames (pid + random) to eliminate concurrent .tmp collisions.

  • writeConfigFile is now a private helper; the public writeConfig export wraps it in the lock, so all write paths are serialized together.
  • setAdapterConfig and deleteAdapterConfig both acquire the lock before their read-modify-write cycle, guaranteeing linearizability within the process.
  • A regression test exercises concurrent writeConfig, setAdapterConfig, and deleteAdapterConfig calls and asserts the final merged state is correct.

Confidence Score: 5/5

Safe to merge — the mutex correctly serializes all write operations, unique temp filenames eliminate concurrent file collisions, and the regression test validates the merged state end-to-end.

The promise-chain mutex is implemented correctly: configMutationLock is always updated to a promise that unconditionally resolves (via the dual () => {} handlers), so no mutation can block the queue even on failure; no deadlock is possible because the private writeConfigFile helper never re-acquires the lock; and each operation reads from disk inside the critical section, guaranteeing it sees the latest committed state.

No files require special attention.

Important Files Changed

Filename Overview
packages/core/src/config-store.ts Introduces a promise-chain mutex (withConfigMutationLock) that correctly serializes all write paths; switches to unique temp filenames (pid + random); refactors writeConfig from a raw export to a locked wrapper over the private writeConfigFile helper. Implementation is correct — the lock chain always resolves (error paths included), no deadlock is possible, and in-process reads outside the lock are safe.
packages/core/src/config-store.test.ts Adds a regression test that fires four concurrent write operations (including a full writeConfig alongside three setAdapterConfig calls) and a second concurrent batch of a delete and a set, then asserts the exact merged state. afterEach correctly restores env and cleans up the temp directory.

Reviews (2): Last reviewed commit: "fix(core): serialize public config write..." | Re-trigger Greptile

Comment thread packages/core/src/config-store.ts Outdated
Comment thread packages/core/src/config-store.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Want your agent to iterate on Greptile's feedback? Try greploops.

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.

1 participant