Skip to content

fix(config): prevent concurrent YAML corruption - #597

Open
Kaguya-19 wants to merge 2 commits into
OpenBMB:mainfrom
Kaguya-19:codex/fix-yaml-concurrent-write
Open

Kaguya-19 wants to merge 2 commits into
OpenBMB:mainfrom
Kaguya-19:codex/fix-yaml-concurrent-write

Conversation

@Kaguya-19

Copy link
Copy Markdown
Collaborator

Summary

  • serialize runtime YAML updates through a stable read/modify/write transaction
  • reject invalid or concurrently edited YAML instead of silently replacing it
  • preserve indentation, key order, comments, and unrelated provider credentials during targeted updates
  • migrate gateway channel, model-test metadata, memory, onboarding, and explicit config saves to conflict-aware atomic writes
  • keep QR sessions retryable when config persistence is temporarily busy

Error behavior

  • explicit stale saves return HTTP 409 with CONFIG_CONFLICT
  • invalid YAML returns INVALID_CONFIG_YAML without changing file bytes
  • background connection-test persistence enters saveError on failure

Tests

  • 5 targeted Vitest files: 181 tests passed
  • targeted ESLint passed
  • git diff --check passed

Coverage includes four-space hand-written YAML, comments and API keys, editor-style multi-step saves, invalid YAML byte preservation, same-length secret-only revision changes, atomic replacement, QR/channel updates, and targeted model metadata persistence.

@Kaguya-19

Copy link
Copy Markdown
Collaborator Author

Addressed both findings in 33a579b:

  • P1: moved the pre-write callback ahead of the final disk revision check and made the final read/check + atomic rename synchronous and adjacent. A deterministic regression test now writes an external version from the callback and verifies CONFIG_CONFLICT while preserving those bytes.
  • P2: atomic writes now resolve PILOTDECK_CONFIG_PATH symlinks and create/rename the temporary file beside the real target. A regression test verifies the symlink remains intact and the target is updated.

Validation: 5 targeted Vitest files, 183 tests passed; targeted ESLint and git diff --check 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.

1 participant