[Bug] Profile cordis.yml is rewritten non-atomically on every boot, so concurrent launches intermittently fail to load

[ivanusto]

Reported as a Discussion because Issues are disabled on this repo.

Summary

Every launch rewrites the profile's cordis.yml in place (O_TRUNC + write, same inode) even when the content is byte-identical. The file is therefore zero-length for a brief window on each boot. A second process starting in that window reads an empty document, fails the top-level-array check, and dies before the agent exists.

This makes dsh --profile headless unsafe to schedule concurrently against one DSH_HOME.

Environment

• @deepseek-ai/dsh 0.1.0-rc.6 (installed from npm)
• Node.js v22.22.1
• Linux (ext4), single machine, one shared DSH_HOME

What happens

Launching two or more headless runs at the same moment against the same profile:

dsh --profile headless "task A" &
dsh --profile headless "task B" &

one of them intermittently exits non-zero at boot with:

Error: dsh: plugin tree failed to load: failed to apply loader entry include (cordis:include):
  failed to validate config file <DSH_HOME>/profiles/headless/cordis.yml

A closely related variant appears when the overlay is read during an unrelated editor save, because that read is not atomic either:

Error: dsh: overlay <DSH_HOME>/profiles/headless/cordis.patch.yml
  must be a top-level YAML array of loader patch entries

Observed roughly 2 failures across ~14 concurrent launches — intermittent, as a race would be. The profile files are intact afterwards, which is what made this confusing to diagnose.

Evidence

The rewrite is in place, not a replacement. cordis.yml keeps its inode and size across a boot while mtime advances, so it is not being written to a temp file and renamed:

before boot: mtime=1786642203  size=223  inode=11562898
after  boot: mtime=1786642867  size=223  inode=11562898

The rewrite truncates first. inotify on the file during one boot:

 0.059s  OPEN         size=223
 0.059s  MODIFY       size=223
 0.059s  MODIFY       size=223
 0.059s  CLOSE_WRITE  size=223
 0.069s  OPEN         size=223

Two MODIFY events between one OPEN and its CLOSE_WRITE is the O_TRUNC-then-write signature (writeFileSync). The final OPEN 10 ms later is another process reading the same file.

The zero-length window is sub-millisecond: polling stat at 1 ms for 25 s (23,693 samples) never observed a size other than 223, so the window is only reachable by a reader that happens to open inside it — which is exactly what concurrent boots do.

Content is unchanged by the rewrite; the file is the stock [] root written at profile initialization.

Reproduction

1. Initialize a profile: dsh --profile headless "reply OK"
2. Launch several at once against the same DSH_HOME:

for i in 1 2 3; do
  dsh --profile headless "Reply with exactly: OK" > run-$i.out 2>&1 &
done
wait
grep -l "failed to validate config file" run-*.out

Intermittent — expect to repeat a few rounds. Adding unrelated load lengthens the boot and widens the window.

Impact

Fail-fast rather than fail-open: the process dies during loader init, before any tool runs, so nothing executes unapproved. The cost is reliability, not safety — a scheduled run is silently skipped unless the scheduler inspects exit codes, and the error text points at a config file that is valid by the time anyone looks at it.

Suggested fix

Write the profile root and any launcher-managed config through a temp file in the same directory followed by rename(2), so a concurrent reader sees either the old file or the new one. Skipping the write entirely when the content is unchanged would also remove the window for the common case, though the atomic write is the more complete fix since it also protects readers during a genuine content change.

The overlay read path deserves the same treatment from the other side: cordis.patch.yml is user-edited, and most editors save by truncating, so a boot racing a save hits the same class of failure.

Workaround

Serializing only the boot phase is enough, since the rewrite happens within the first moments of startup:

exec 9>"$HOME/.dsh-boot.lock"
flock 9
dsh --profile headless "$TASK" &
child=$!
sleep 6          # cover the rewrite; the model-heavy remainder still overlaps
flock -u 9
wait "$child"

With this in place, the concurrent runs that previously lost one launch out of three complete reliably.

[naipi11]

不用等修复，同一份配置目录（同一 DSH_HOME / 同一 profile）不要并发启动多个实例：

1. 一次只启动一个实例；同时开多个时，把启动阶段串行化——用 flock/锁文件占位，或者等第一个实例完全起来之后再启动第二个。
2. 启动失败就重试一次：失败时配置文件事后仍是完好的（写入走临时文件加重命名，要么成功要么报错，不会留下半截内容），直接重启即可。
3. 平时注意别让多个入口同时触发启动（例如托盘常驻之外又手动开了一个）。

根因是每次启动都会把组合配置写回配置文件，写入用的是固定同名的临时文件再加重命名，进程之间没有锁；两个进程同时启动就会互相覆盖同一个临时文件并竞争重命名，偶发报加载失败。等官方改成带锁或随机后缀的原子写之后，并发启动才安全。

English: Don't start two instances on the same DSH_HOME/profile at once; serialize the startup phase (flock or wait until the first is fully up), and since the config file stays intact after a failed boot (tmp-file plus rename never leaves a partial file), just retry once.

[ivanusto]

Thanks for the workaround — the serialization advice matches what I ended up doing (flock around just the boot window; no recurrence in four days of scheduled concurrent runs).

That said, I don't think the write path you described is the one that runs at boot. There are two writers in the published package:

1. @deepseek-ai/cordis-plugin-include/lib/index.js:243 — _writeFile(), the tmp-plus-rename one you're describing: writeFile(this.filename + ".tmp") then rename(...), fixed suffix, no lock.

2. @deepseek-ai/dsh/lib/profile-boot-DG5t9aNs.js:143 — prepareProfile():

writeFileSync(join(profile.dir, PROFILE_ROOT_FILENAME), PROFILE_ROOT_CONFIG); // PROFILE_ROOT_FILENAME === "cordis.yml"

No temp file, no rename, no lock, executed unconditionally on every boot. This is the boot path, and it is what the report is about.

The observable difference is the inode: a rename replaces it, an in-place write keeps it. Measured across a single boot on 0.1.0-rc.6:

before: inode=11562898 size=223 mtime=1787004002
after:  inode=11562898 size=223 mtime=1787026139

Same inode, same size, mtime advanced — and it has been the same inode across a week of daily scheduled boots. So the hazard is not two processes racing on a shared .tmp name; it is O_TRUNC on the live file while another process reads it. That is why the reader sees an empty document, fails the top-level-array check, and the file is intact afterwards.

Two consequences:

• Making the temp write atomic in (1) — random suffix, or a lock — will not fix this report, because the boot path never goes through (1).
• The bytes written by (2) are a compile-time constant (PROFILE_ROOT_CONFIG), so every boot rewrites the file with content it already has. The siblings created nearby are already guarded:

if (!existsSync(patchPath)) writeFileSync(patchPath, PROFILE_PATCH_TEMPLATE);
if (!existsSync(workspacePath)) writeFileSync(workspacePath, PROFILE_PNPM_WORKSPACE);

Guarding cordis.yml the same way closes the window entirely, with no lock needed.

Checked against 0.1.0-rc.7 (published 2026-08-17): profile-boot-DG5t9aNs.js:143 is byte-identical, so this still applies to the current release.

Minor and separate: the rename retry in (1) treats only EACCES/EBUSY/EPERM as retryable, so if two writers ever do race there, the loser's ENOENT propagates instead of being retried.

[naipi11]

对，启动那条路我说错了。前面写的 tmp + rename 是 include 写回，不是 boot。

boot 走的是 prepareProfile() 里对 cordis.yml 的无条件 writeFileSync：同一 inode，先截断再写入那段固定的空根配置。所以另一个进程会读到空文档、过几秒文件又完好。把 include 那条改成随机后缀或加锁，确实修不到这个报告。

用户侧做法不用改：同一 profile 还是只把启动阶段串行（flock 包住 boot 窗口就够），失败直接重试。官方那边更干净的修法是像旁边的 patch / workspace 一样，文件已经在就不写；内容本来就是编译期常量。

English: Agreed — the boot writer is the unconditional writeFileSync in prepareProfile, not the tmp+rename path. Serialization still works; skipping the rewrite when cordis.yml already exists would close the window without a lock.

[ivanusto]

Thanks for going back and checking — that matches what I found, and your fix shape is better than a lock.

Operational data since the report, in case it is useful for prioritising: the flock-around-boot workaround has now covered 207 headless launches (195 benchmark runs plus 12 scheduled daily inspections) across seven days, with zero failed to validate config file failures. Serialising only the boot window costs nothing measurable, because the lock is released a few seconds in and the model-heavy remainder still overlaps. So from a user's perspective this is fully worked around, and the report is about the default being sharp rather than about being blocked.

On the fix: skipping the write when cordis.yml is already there does close the window, and the precedent next door makes it the natural shape. One wrinkle worth deciding deliberately — write-if-absent never refreshes the file, so if that root document ever changes between releases, an existing profile keeps the old one indefinitely and the difference only shows up as behaviour that does not match the installed version.

Writing it the way the include path already does would avoid having to choose:

• compare first, and return without touching the file when the content already matches — that is the common case on every boot, and it removes the truncate entirely;
• when it does differ, write to a uniquely-named temporary file and rename() over the target.

That is atomic for the rare upgrade case and a no-op for every ordinary boot, so concurrent launches never observe a partial document, with no lock and no staleness. It also means the two writers in the profile directory work the same way, which is one less thing to know.

Happy to open a PR for that shape if it is acceptable — it is a small change and I have a reproduction environment that exercises the boot path a few hundred times a week.

[ivanusto]

Correction to my last comment: I offered to open a PR without checking CONTRIBUTING.md, which says external pull requests are not accepted right now. Understood — leaving the branch here instead, so it costs the team a cherry-pick rather than an implementation if the shape is useful.

Branch: ivanusto/deepseek-harness@fix/profile-root-config-atomic-write (8a493ae)

What it does. prepareProfile compares before writing. The ordinary boot finds the content already correct and writes nothing, so the truncate window does not exist on that path. When the content does differ, the replacement goes through a new writeFileAtomicSync in @deepseek-ai/dsh-atomic-write — exclusive-create temp sibling, caller's mode on the fresh inode, rename commit, temp removed on failure.

Why compare rather than write-if-absent. This is the part I would have got wrong if I had implemented your suggestion literally. prepareProfile's own docstring gives the reason the rewrite is unconditional: the Loader's tree write-back can persist composed rows into cordis.yml, and leaving those in place would duplicate every bundle insert on the next boot. Write-if-absent closes the race but drops that guarantee. Comparing content keeps both — a file that has been written over still differs, so it is still restored — and costs one read on a path that currently always writes.

A sync sibling rather than await writeFileAtomic: boot writes its config before the event loop carries any work, so making that path async would spread through every caller of the boot sequence for no behavioural gain. Hand-rolling the replacement inside profile-boot.ts seemed worse — duplicated logic that would drift from the async version, and the clone-detection gate would likely flag it.

Verification, on Linux with Node 22.22.1:

• pnpm vitest run on both new/changed test files — 17 passed. writeFileAtomicSync is covered on the same paths as its async sibling (creation with parents and exact mode, narrowing a wider-permission file, replacing a symlinked target rather than writing through it, dirMode, no temp left behind when the rename fails). The comparison is covered by unchanged inode and mtime when content matches, restoration when the file holds baked rows, and a changed inode proving the update arrived by rename rather than truncation.
• pnpm run lint, pnpm run typecheck — clean.
• pnpm run doc-sync — 28 gates, 0 failed.
• Not run: the full suite, test:coverage, e2e, snapshots.

It includes an Agent Note under implemented/bug-fix/ with the Chinese counterpart and the pairing record, since the repo requires one for a behaviour change.

Nine files, +300/−9, of which the Agent Note and tests are most of it; the behavioural change is about fifteen lines.

No urgency from our side — the flock workaround has now held for 207 headless launches over seven days.