Skip to content

feat(natives): durable Windows replacement for Telegram publication - #3606

Closed
twoimo wants to merge 1 commit into
Yeachan-Heo:devfrom
twoimo:feat/natives-durable-replace-path
Closed

feat(natives): durable Windows replacement for Telegram publication#3606
twoimo wants to merge 1 commit into
Yeachan-Heo:devfrom
twoimo:feat/natives-durable-replace-path

Conversation

@twoimo

@twoimo twoimo commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Hello, and thank you for taking the time to look at this.

This adds one native primitive together with the consumer that needs it, so the new export is not authority code sitting on its own.

The problem

writeJsonAtomic in telegram-daemon.ts stages a sibling temp file, flushes its bytes, and finishes with fsImpl.rename(tmp, file). On Windows that rename does replace the destination, but the namespace mutation itself is only recorded in the volume's write-back cache. A crash immediately after an acknowledged daemon-state or topic-registry write can therefore lose it. The bytes were flushed; the replacement that published them was not.

The change

durableReplacePath(sourcePath, destinationPath) issues MoveFileExW with MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, which does not return until the replacement is on stable storage.

The typed result distinguishes outcomes rather than collapsing them to a boolean:

  • mutationState: committed, not_committed, or unknown
  • durabilityState: durable, not_attempted, or not_provable

not_committed is only reported when both source and destination identities were observable before the attempt and are unchanged after it, so a caller can retry safely. Anything less certain is unknown. Non-Windows hosts return unsupported_platform without touching the filesystem.

Consumer and fallback

The replacement step is factored out of writeJsonAtomic into an exported replaceStagedPublication(fsImpl, staged, destination, platform?, durableReplace?). platform and the binding are injectable, so the Windows branch is reachable in tests on any host — the acceptance surface does not depend on CI having a Windows runner.

Availability is deliberately never reduced. A missing binding, a throwing binding, or an outcome proving the replacement did not commit all fall back to the previous rename, so publication is no less available than before this change. Fallback is refused in exactly one case: when the outcome cannot prove the replacement did not land. Renaming again there could resurrect the staged file over a newer successor, so that surfaces as an error instead.

Verification

  • packages/coding-agent/test/notifications-telegram-durable-replace.test.ts — 5 pass. Covers: non-Windows never calls the binding; Windows success skips the rename; a not_committed outcome falls back; an unknown outcome refuses to fall back and throws; a throwing binding falls back.
  • bun test packages/coding-agent/test/notifications-telegram-daemon.test.ts — 512 pass, 0 fail.
  • bun test packages/natives/test/path-identity-windows.test.ts — 31 tests, correctly skipped on a non-Windows host.
  • tsc -p packages/coding-agent/tsconfig.json --noEmit — clean.
  • biome check — clean on all touched files.
  • cargo check -p pi-natives — clean.

The Windows-only Rust body cannot be compiled on my macOS host; it is behind #[cfg(windows)] and I am relying on the Windows native-build job here. Please do tell me if you would rather that path were exercised differently.

Rebased onto the current dev tip so the exact head is what CI measures. Thank you again for the review.

…tion

Add a Windows write-through replacement primitive and wire it into the
Telegram daemon's staged-publication path.

`durableReplacePath(source, destination)` issues `MoveFileExW` with
`MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH`, so the namespace
mutation is on stable storage before the call returns rather than sitting
in the volume's write-back cache. The typed result reports `mutationState`
and `durabilityState`, so a caller can tell a replacement that provably
did not happen from one whose outcome cannot be proven. Non-Windows hosts
return `unsupported_platform` without touching the filesystem.

The consumer is the Telegram daemon's own `writeJsonAtomic`, which
previously finished with a plain rename and never flushed it. The
replacement step is factored into an exported `replaceStagedPublication`
seam that takes an injectable platform and binding, so the Windows branch
is reachable in tests on any host.

Availability is never reduced: a missing binding, a throwing binding, or
an outcome that proves the replacement did not commit all fall back to
the previous rename. Fallback is refused only when the outcome cannot be
proven, because retrying there could resurrect the staged file over a
newer successor.

@Yeachan-Heo Yeachan-Heo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES — terminal hostile review of exact head 79e612957837f66c5900535329d24df823d327f9 against exact base dd26a0787260b4c54b6298b2f46b4908b2b1ed1a and the active sole-owner #3596 contract.

P1 — protected native authority was added without a synchronized semantic manifest/generation attestation. Exact-head run 30576024783, generation job 90984521458, fails: semantic manifest declaration digests do not byte-match the current tree. Log SHA-256: 46feca831386692e79e83a25bae734cfa87444459421ad0f7088a25b8d3fe05c. This is not an aggregate presentation issue; the primary protected-authority gate is red.

P1 — the Windows-only implementation does not compile on Windows. Exact-head Windows job 90984521471 fails with seven E0422/E0425/E0433 errors at path_identity.rs:5754-5828: NativeDurableReplaceResult is absent from the Windows platform module scope. Log SHA-256: cd92f9101688fad05405f9d3bb6cc25c5a1fdcc5e1c2502dce0771ff38f33bd7. Merely importing the type would clear only the first compiler symptom, not the authority blockers below.

P1 — the new two-path primitive is identity-light and its fallback is not commit-safe. durableReplacePath(sourcePath, destinationPath) accepts no caller-persisted source/destination identity or digest. Its not_committed proof compares only path-presence and volume/file index before/after MoveFileExW; an in-place destination content change preserves those identifiers, so the consumer may fall back to fs.rename and overwrite a newer destination. More seriously, replaceStagedPublication falls back after any thrown native call, although a throw is not proof that MoveFileExW did not commit; a staged-path successor can then be renamed over the durable destination. On success the native verifier checks only that the destination is a file, not that it is the authorized staged source bytes.

This materially duplicates and weakens the active #3596 authority: #3596 already owns crates/pi-natives/src/path_identity.rs and packages/natives/native/index.d.ts, exports guarded exactReplacePath/exact_replace_path, requires staged source and destination identities/digests, retains verified source recovery authority, performs final digest checks, and advances guard/generation evidence. Adding a required loader capability for a weaker parallel replacement primitive obscures that sole-owner contract and is not independently necessary.

CI is already decisively merge-blocking. Closing this external PR unmerged now under doctrine; remaining jobs are not rerun or cancelled, the contributor branch is untouched, and no duplicate repair lane is created.


[repo owner's gaebal-gajae (clawdbot) 🦞]

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.

2 participants