fix(etc_cmdlog): store the command log language-neutral, localize at read-time (#460)#468
Merged
Conversation
…read-time (#460) The onBroadcast writer baked the localized labels (msg1/msg2) into each stored line, and both readers dumped the file raw - so every entry was frozen in whatever language was active when it was logged. A hub whose cfg.language changed showed a mix, and a German hub whose entries all predated the switch printed English on `+cmdlog show`. Entries are now stored language-neutral: four fields (ts, cmd, args, nick) separated by the US control byte (0x1f). One shared render_line applies the current msg1/msg2 at read-time, on both the ADC and the HTTP path, so old and new entries render in the hub's current language. Migration is automatic: an old baked line has no delimiter and is returned as-is (its frozen language) rather than dropped or mis-parsed - the delimiter's presence is the format marker. Every field is run through util.strip_control_bytes at write-time, which keeps the delimiter out of any field and also closes a latent corruption where a newline in a command's args split one entry across two lines. The write open is now nil-guarded so a missing log/ dir can't crash the onBroadcast listener. GET /v1/log/cmd keeps its {lines, returned, total_lines} contract; its lines are simply re-localized now instead of frozen - no HTTP_API.md change. etc_cmdlog v1.4 -> v1.5. Provably fails pre-fix (§1a.7): tests/unit/etc_cmdlog_test.lua (both smoke.yml legs) writes under en, switches the hub to de, and asserts `+cmdlog show` renders the German labels - 6 of 17 checks are red on v1.4, all green on v1.5. Covers the old-line raw fallback, a mixed file, redacted-arg storage, control-byte stripping, exact-spacing freeze, and the HTTP path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #460.
On a German hub,
+cmdlog showprinted English. Root cause: theonBroadcastwriter baked the localized labels (msg1/msg2) into each stored line, and both readers (+cmdlog showandGET /v1/log/cmd) dumped the file raw. So every entry was frozen in its write-time language, a hub whosecfg.languagechanged showed a mix, and entries predating a switch never re-localized.Fix
ts,cmd,args,nick) separated by the US control byte (0x1f). No labels on disk.render_lineapplies the currentmsg1/msg2, on both the ADC and HTTP path, so old and new entries alike render in the hub's current language.render_linereturns it as-is (its frozen language) instead of dropping or mis-parsing it. The delimiter's presence is the format marker.util.strip_control_bytesat write-time, which keeps the delimiter out of any field and also closes a latent corruption where a newline in a command's args split one entry across two log lines. The write open is nil-guarded so a missinglog/dir can't crash theonBroadcastlistener.etc_cmdlogv1.4 -> v1.5.Design notes
dkjsonis an optional build dep (core/init.lua: the HTTP router refuses to bind if it did not load), so a core logging plugin must not hard-depend on it. A control-byte delimiter needs no lib and, becausestrip_control_bytesreplaces every%c, can never appear inside a field.GET /v1/log/cmdcontract unchanged ({lines:[string], returned, total_lines}) - its lines are simply re-localized now instead of frozen, so nodocs/HTTP_API.mdchange was needed. Returning structured fields there (the issue's other suggestion) is a possible future improvement, deliberately deferred to keep this a non-breaking language fix (per the AskUserQuestion decision).Tests (§1a.7)
New
tests/unit/etc_cmdlog_test.lua, registered on both smoke.yml legs. It writes an entry underen, switches the hub tode, and asserts+cmdlog showrendersBefehl:/benutzt von:. 6 of 17 checks are red on the unpatched v1.4 (the labels are baked English) and all 17 pass on v1.5. Also covers: old-line raw fallback, a mixed-language file, redacted-arg storage never writing the secret, control-byte stripping (no extra log lines), an exact-spacing freeze, and the HTTP path.Review (§1a.6)
Independent reviewer (correctness / security / edge-cases / test quality) + maintainer spot-check: no BLOCKERs, no CONCERNs. Confirmed empirically that v1.5's render is byte-identical to v1.4's stored string, that control-byte stripping makes field/line injection impossible, and that the test genuinely discriminates. Both optional NITs folded in: the writer nil-guard and an exact-equality spacing assertion. Verified no other plugin shares the unguarded-append-writer pattern (§1a.1).
3.2.x only, not backported.
🤖 Generated with Claude Code