Repository navigation
Do not let a failed state write drop a message (#1096) - #1132
Conversation
The friend-line mark and the exchange record are both written before the agent has the line, so a rejected mutate() (the lock was taken over, or the disk refused) used to fail the whole delivery and lose a message that exists nowhere else. Both writes now go through recordOrWarn: one warning names what could not be recorded, and the line is still handed over. The two new cases force rt.mutate to reject; both are red on the previous tree (0 of 2, deliver() rejected) and green here with exactly one warning each.
|
Verdict: PASS at 2a09e5f Written-by: Reviewer Judged on the merge tree: the six files onto The change
Ran: the new cases, the full core suite on the merged tree, a pristine The 42 are byte-identical between the two runs ( Knives, each on a copy of the merged tree: Both halves of the acceptance are load-bearing: reverting the source reddens "still delivered", removing the warning reddens "exactly one warning". The hand-written Non-blocking
|
Owner: Dev3
Reviewer: Reviewer
Written-by: Dev3
Fixes #1096
Tip SHA: 2a09e5f
Reproduce
deliver()writes twice before the agent has the message:noteFriendLine(the friend-line mark,core/src/host/delivery.js)and the exchange record push. Both were made
await rt.mutate(...)by#1048, andmutate()rejects when the lock wastaken over or the disk refuses — so today a failed write fails the delivery, and the line is dropped. It never existed
anywhere else: neither write is a copy of a message that is already stored.
New cases force it:
rt.mutate = async () => { throw ELOCKLOST }, then deliver a friend's line and, separately, areplyexchange; each case asserts the agent still received the message and that exactly one warning was printed.What changed
core/src/host/delivery.js: both writes go through one small helper,recordOrWarn(what, write)— try to write,on failure print one
rt.warnline (ds-bot: delivery: <what> was not recorded: <error>) and continue. Nothing elseabout
deliver()moved; the live-holder path (save()warns, never throws) is unchanged.core/test/delivery-write-failure.test.js(new): the two cases above, on a loaded runtime with the agent recordingwhat it is handed.
patches/ds-bot/0005-social-seams.patch+patches/ds-bot/files/test/delivery-write-failure.test.js: the mirror ofthe two files, made by hand because this box has no
git(see "Not run").docs/subsystems/core-host.md: one sentence in the one-writer-per-home bullet — a failed write does not decide thedelivery.
changelog.d/1096-delivery-write-failure.fixed.md: a message no longer disappears when the write fails.What I ran
cd core && node --import ./test/harness-resolve.mjs --test test/delivery-write-failure.test.js→ 2/2.delivery.js(restored from the base tarball) → 0/2, both red("the line is handed to the agent instead of being dropped"), then restored with
cmp.test/delivery-write-failure.test.js test/mutate-writers.test.js test/bot-messages.test.js test/repeated-delivery.test.jstest/blocking-question.test.js→ 40/40.node scripts/ds-bot-patches.mjs check→skipped pin (no git)/ok 10 patches apply cleanly in order/ok core/ equals ds-bot/ + patches/ds-bot/.node .github/scripts/changelog.mjs check→ 33 fragments well formed.diff -rqagainst a pristined797cbe4unpack → exactly the six files above (plus the harnessnode_modulessymlink).Red without the source change
Yes — that is the point of this change and the reason it is not just a test PR: with today's
delivery.jsthe two casesare 0/2, because
deliver()rejects instead of handing the line over. The knives from the acceptance hold too:swallow the rejection with no
warnand the "exactly one warning" assertion goes red; rethrow and the "still delivered"assertion goes red.
Not run
node scripts/ds-bot-patches.mjs generate— it needsgit, which this box does not have. I wrote the0005sectionwith
diff -uplus the generator's own header rules (old-side start as the hint, noindexline) and placed the newtest file whole under
patches/ds-bot/files/;check— the strict applier that CI also runs — applies all tenpatches and reproduces
core/byte for byte.npm run verify,oxlint,npm run build— no oxlint binary or esbuild here; CI'stests-okandNode 24jobscover lint, the client build check and the full suite.
delivery.js, exercised by the newcases and
bot-messages.Risk
Low, and one-sided: the only behaviour that changes is that a write failure no longer loses the message. A failure is
still visible exactly once (no silent swallow), and the record genuinely stays unwritten — the next successful write
re-does it if it is a mark, and a lost exchange stays lost by design, since it is a note about the line, not the line.