fix: NBT serialization crash in ArchieFluidSlot/Storage and ArchieEnergyStorage - #9
Merged
Merged
Conversation
…gyStorage
Found by running ./gradlew check locally (the user asked to look at the
latest run - GitHub's runner was mid-flight, so this ran it locally
instead), which crashed on the new test added in the docs-sync work:
net.benwoodworth.knbt.internal.NbtEncodingException:
Cannot encode TAG_Compound within a TAG_List of TAG_Long
ArchieFluidSlot.Serializer and ArchieFluidStorage.Serializer both declared a
multi-field class-shaped SerialDescriptor but implemented serialize()/
deserialize() as flat, unwrapped sequential encoder/decoder calls instead of
encoder.encodeStructure/decoder.decodeStructure. Harmless for a single slot
in isolation, but breaks the moment a slot holding real (non-blank) content
sits inside ArchieFluidStorage's list: knbt commits to a homogeneous list
element shape from the first raw encodeLong call, then chokes when the same
slot's serialize() call goes on to write a compound-shaped resourceStack.
Any real mod filling an NBTHolder.fluidField(...) tank and triggering a save
would hit this - it wasn't specific to the test, just nothing had exercised
that path with a non-empty fluid before.
Fixed both to properly wrap their encode/decode in encodeStructure/
decodeStructure. Found the identical pattern (descriptor says structured,
implementation writes flat) in ArchieEnergyStorage's serializer too - not
currently triggered (energyField is never list-nested), but the same latent
defect, so fixed for consistency while in the area.
Verified: the specific failing test now passes, and a full local
`fabric:runGametest` server-side suite run is green.
Also investigated a separate, pre-existing client-GameTest flakiness (not
caused by this fix - confirmed via 5x repeated local runs, 4 passed, and by
comparing against pre-existing CI failures on unrelated PRs). Documented
what was found and ruled out in AGENTS.md rather than guessing at a fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
KP2048
marked this pull request as ready for review
August 7, 2026 02:11
4 tasks
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
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.
Summary
Found by running
./gradlew checklocally (asked to look at the latest run of:check; the GitHub-hosted run was still in progress, so ran it locally instead). It crashed:ArchieFluidSlot.SerializerandArchieFluidStorage.Serializerboth declared a multi-field, class-shapedSerialDescriptorbut implementedserialize()/deserialize()as flat, unwrapped sequential encoder/decoder calls instead ofencoder.encodeStructure/decoder.decodeStructure. Harmless for a single slot in isolation, but breaks the moment a slot holding real (non-blank) content sits insideArchieFluidStorage's list: knbt commits to a homogeneous list-element shape from the first rawencodeLongcall, then chokes when that same slot'sserialize()call goes on to write a compound-shapedresourceStack. Any real mod filling anNBTHolder.fluidField(...)tank and triggering a save would hit this — it wasn't specific to the new test, just nothing had exercised that path with a non-empty fluid before.Fixed both to properly wrap encode/decode in
encodeStructure/decodeStructure. Found the identical pattern inArchieEnergyStorage's serializer too — not currently triggered (energyFieldis never list-nested), but the same latent defect, fixed for consistency while in the area.Also investigated a separate client-GameTest flakiness found during this same
:checkrun (not caused by this fix — confirmed via 5x repeated local runs and comparison against unrelated pre-existing CI failures). Root-caused it as far as static analysis allows and documented findings inAGENTS.mdrather than guessing at a fix; a real fix (bringing the harness's coroutine scope in line with how real Jetpack Compose's own test tooling avoids this class of race) is scoped as a separate follow-up PR.Test plan
BlockEntityNBTHolderTests.testItemFluidAndEnergyFieldsPersistMutations) now passesfabric:runGametestserver-side suite: green:common:compileKotlinclean🤖 Generated with Claude Code