fix(tests): write migration output back to the file that was read

[kaceper11]

What this fixes

An intermittent full-suite failure simion saw while verifying #353:

agent_hooks::tests::a_clones_status_line_is_read_from_its_own_config_dir fails ~once per cargo test run, passes alone, pre-existing on main.

The mechanism

with_scratch_data_dir serializes tests that redirect TERMIC_DATA_DIR, but TERMIC_DATA_DIR is process-global: tests that read global_dir()-derived paths without the lock can still straddle a neighbour's scratch window.

That alone is a known hazard — but it produces writes through one specific path: the migration write-backs.

Both load_settings_in and load_projects_in resolve the record file once for the read, run their migrations, and then call save_*_in(id, …) — which re-resolves global_dir() a second time at write time. If a scratch window opens in the gap, the write lands in a directory the load never read from:

• Foreign load_settings_inner resolves env → real/fixture path, reads a legacy-shaped settings.json, sets migrated → write-back re-resolves → a scratch window just opened → the migrated copy lands over that scratch's settings.json, wiping the agent the victim test just saved → its next-claude lookup falls back to ~/.claude → assert fails.
• Reversed the same way: a load that resolved inside a scratch can write the migrated fixture into the developer's real data dir if the window closes mid-write.

Why not lock the accessor instead

Gating global_dir() (or the settings accessors) on DATA_DIR_LOCK deadlocks: PORT_ALLOC_LOCK is held across task-create flows that call load_settings_inner, while scratch closures call task-creation paths that need PORT_ALLOC_LOCK.

The fix

Both write-backs now write to the path already resolved for the read (save_settings_at(&f, …) / save_projects_at(&f, …)), which is what their own comments already claim — "the write has to land in the profile the records came from". save_settings_in/save_projects_in keep their signatures for explicit saves.

Honest caveats

• I could not reproduce the flake locally (6 clean full-suite runs); it is ~1-in-N upstream. This removes the one mechanism found by auditing every set_var/save_settings_*/global_dir writer that can inject a foreign settings.json into a live scratch without an unlocked writer call.
• Residual, unaddressed: unlocked readers still see whatever TERMIC_DATA_DIR points at mid-window (wrong data for their asserts — the "different test each time" signature from the comment in test_support.rs). Closing that needs the accessor gating ruled out above, or dropping the process-global seam entirely.

Test plan

• cargo test — 1216 pass on this branch
• Race window itself is not unit-testable without a seam between resolution and write

[simion]

Heads up, this branch now has a second commit (cfd95ce, "feat(delivery): add Delivery panel for agent handoffs") on top of the flaky-test fix that was already reviewed and approved. The diff jumped from 61 to ~4100 lines and the title/description still only describe the test fix. Can you split the delivery panel into its own PR? Happy to review the test fix as-is once the branch is back to just that.

[simion]

Could you split this one? As it stands the PR is titled and described as a 23-line test fix, and carries a 4,099-line feature nobody has reviewed:

• eef0cbba fix(tests): migration write-back, 1 file, +23/-4. This is the one the description covers.
• cfd95ce3 feat(delivery): add Delivery panel for agent handoffs, 36 files, +4,099/-65. Not mentioned anywhere in the body.

I would merge the first today. It is a better diagnosis than the one I pushed: I treated that flake as my own unsynchronised set_var in the linux_desktop tests and made them take DATA_DIR_LOCK, which removes one hazard but not the one you found. load_settings_in and load_projects_in re-resolving global_dir() at WRITE time is a real bug in production code, not a test problem, and the write landing in a directory the read never came from is exactly the shape of the failure. Worth retitling it as such: "fix(tests)" undersells what it repairs.

Worth saying that neither of us has reproduced it. You got 6 clean full-suite runs, I got 8 after my change. Two unproven fixes for one flake, and your caveats section is the right way to say so.

The Delivery panel should stand on its own PR with its own description. It is not that I object to it, it is that I cannot review what the body does not mention, and at 36 files it is the kind of change that wants its own reasoning written down. Same request as #352, one step further: there at least the body said it was stacked.

If the two are genuinely entangled, say so and I will read them together, but from the commit boundaries they look independent.