Skip to content

fix(gateway): preserve user comments when persisting startup config migrations - #1881

Open
Kiuyor wants to merge 1 commit into
TokenRhythm:mainfrom
Kiuyor:fix/config-migration-preserve-comments
Open

Kiuyor wants to merge 1 commit into
TokenRhythm:mainfrom
Kiuyor:fix/config-migration-preserve-comments

Conversation

@Kiuyor

@Kiuyor Kiuyor commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Scope

Scope boundary: src/opensquilla/gateway/config_migration.py(单文件)+ 两个测试文件。启动期配置迁移的落盘从"整档重排"改为"先走仓库自带的保注释 patcher":

  • 新增 atomic_replace_bytes(target, data):把 atomic_write_config 里字节落地那一步(同目录临时文件、fsync、0600、os.replace)抽出来共用——崩溃可持久化契约仍只有一份;atomic_write_config 改为 tomli_w.dumps 成字节后调它(字节与旧 tomli_w.dump 逐字节等价,已实测)。
  • ConfigMigrationResult 增 original 字段(load 时校验过的迁移前 payload;全仓构造点仅一处);三个门控点与全部调用签名零改动(gateway/config.py 0 改动)。
  • backup_and_write_migrated_config:先 patch_import_config(盘上字节, original, payload) 保注释改写;patcher 抛 LosslessTomlPatchError 时回落全量重排,并在结构化日志记 rewrite: "comment-preserving" | "full-reserialize"(回落时另带 lossless_error)。

Non-goals: 不改配置值的迁移语义;不改调用侧;不动 onboarding 的合并路径(onboarding/config_store.py:975 那条是合并、不是迁移);不给 patcher 增加新的表达能力。

Branch

Base branch: main

Target exception: N/A

Issue

Linked issue: Fixes #1872

If None, reason: N/A

Release Note

Release note: 启动期配置迁移不再整档重排 config.toml——只改写真正变化的行,用户的注释与排版保留;仅当无法无损改写时才回落到旧的整档重写(备份照旧,日志注明)。

Tests

Ruff: uv run ruff check src tests — All checks passed

Pytest: 新增两条,另跑 CI 同款两相全量——并行相(-n 8 --dist loadfile):31983 passed / 728 skipped / 38 failed(31983 = 基线 31981 + 新增 2;38 条失败集合与 4494195b9 基线上同机跑出的清单逐条一致,全部为本机环境类或上游自带红,见下方 details);串行相(ci_serial):313 passed / 5 skipped / 0 failed。新增覆盖:a) 完整启动迁移路径保注释(三种注释形态存活 + 落盘字节 == patcher 输出);b) 盘上字节被外部改动时回落全量重排(结构化日志 rewrite=full-reserialize + lossless_error)。

Build: npm --prefix opensquilla-webui run build — 通过(Web UI artifact verified: 156 files,已按仓库流程 stage 给打包);uv build --wheel — 成功(opensquilla-0.5.6-py3-none-any.whl)

Regression tests: added

Notes: 为什么这里回落而不是像三处既有 patcher 调用方那样 fail-closed——这条路径在启动加载链上,"补丁打不上"的替代行为是"整档被重排"(本次改动前的既有行为),而不是"半迁移的导入";两者代价不对称,所以保留可用性并显式记录。该理由已写进函数 docstring。

The default test path remains offline, deterministic, credential-free, and safe for forks.

本机全量的环境类失败说明(38 条与 4494195 基线上同机跑出的集合逐条一致,零条与本次改动相关)

Maintainer Live Check

Maintainer live check: no

Surface: N/A

Maintainer-only note: contributors are not expected to provide secrets or run credentialed live checks. Maintainers may run Live Release E2E for provider, browser, gateway, channel, or release smoke coverage.

Safety

Secrets, local-only artifacts, private prompts/transcripts, channel identifiers, AI session artifacts, non-public fixtures, and tests/_private/ contents must not be committed.

Third-Party Origin

Third-party origin: none

Details if non-none: N/A

Documentation Changes

  • Links point to existing repository files or stable external pages.
  • Code fences and Markdown tables render correctly on GitHub.
  • Examples avoid real secrets, local private paths, and private transcripts.

The startup load path rewrites a changed config.toml with tomli_w.dump,
which reserializes the whole document and silently drops the user's
comments and layout. The repository already ships a comment-preserving
patcher used by three other import paths; this wires it into the
migration write:

- Extract atomic_replace_bytes so the crash-durability contract
  (same-dir temp, fsync, 0600, replace) stays in one place;
  atomic_write_config serializes and calls it.
- Carry the pre-migration payload on ConfigMigrationResult so the
  write path can patch from the exact validated bytes to the new
  payload.
- backup_and_write_migrated_config now patches first; on
  LosslessTomlPatchError it falls back to the previous whole-file
  rewrite and records which rewrite ran in the structured warning.

Tests: the full startup migration path keeps all three comment forms
(byte-equal to the patcher output); moved-on disk bytes exercise the
fallback with rewrite=full-reserialize in the structured log.
@Kiuyor

Kiuyor commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

CI 结果说明(供参考):本 PR 的 20 pass / 3 fail / 13 skip 中,两处红都已在改动外定性——

  1. Windows high-risk (desktop-installer-contracts-2):tests/test_ci/test_workflows.py::test_release_jobs_share_one_rerun_stable_verified_webui_artifact - assert False。这是本 PR 之外的既有红——main 自身的 run 36785179481(9-30)同样红在这条(其 ubuntu offline 分片同族也红)。成因:fix(release): validate existing Draft upgrades without rebuilding #1871 在 wheelhouse-release.yml 新增的两个 upload-artifact@v4 步缺 overwrite: true,该提交以 [skip ci] 合入未跑全量 CI;已另开 [Bug]: 4494195b9 上 test_release_jobs_share_one_rerun_stable_verified_webui_artifact 必红:#1871 新增两个 upload 步缺 overwrite: true([skip ci] 未跑 CI) #1880。
  2. Windows high-risk (core-2):tests/test_tools/test_shell_blocking_launch.py::test_slow_native_launch_preserves_loop_and_ownership[stdin-cancel] - TimeoutError。时序敏感用例(测试自带 assert release.wait(5) 与 wait_for(..., timeout=10)),负载 runner 上超时;同分片兄弟参数 [stdin-fail] 与全文件其余用例全过、main 当日同分片 success,与本改动(配置落盘字节路径)无交集。建议 rerun 该 job。

本改动自身:本地 CI 同款两相——并行相 31983 passed(38 条失败与 4494195b9 基线同机清单逐条一致,即上述环境类/上游红集合),串行相 313 passed / 0 failed。

This branch has not been deployed

No deployments
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.

[Bug]: 启动期配置迁移整份重写 config.toml,静默丢弃用户注释——仓库自带的保注释补丁器未接上这条路径

1 participant