Skip to content

test(archive-codec): reach a 100% mutation score - #1264

Merged
Mearman merged 42 commits into
mainfrom
feat/100-percent-mutation-archive-codec
Sep 13, 2026
Merged

Mearman merged 42 commits into
mainfrom
feat/100-percent-mutation-archive-codec

Conversation

@Mearman

@Mearman Mearman commented Sep 12, 2026

Copy link
Copy Markdown
Member

Works archive-codec's Stryker mutation score from its measured baseline of 76.26% to a genuine 100%, killing every survived/no-coverage mutant by fixing real test gaps, restructuring code so equivalent-mutant opportunities don't exist as AST nodes, and correcting pre-existing test bugs surfaced along the way. Zero Stryker disable comments.

Final verified state (local stryker run stryker.config.ts, config file passed explicitly): 100.00% mutation score, 0 survived, 0 no-coverage, across every file in the package (cfb, crypto, oleps, test-support, zip, magic.ts). breakThreshold raised to 100 accordingly.

…boundary coverage

splitBitLength64 is exported from md5.ts and used by padMessage via
DataView.setUint32 writes instead of a hand-rolled per-byte
shift-and-mask loop, so RFC 1321's >2^32 bit-length boundary (reached
only by a message past 512 MiB) is directly testable without hashing
an actual 512 MiB buffer.
…check

Indexing a Uint8Array past its own end reads back undefined, which can
never equal one of magic's numeric entries, so a bytes shorter than
magic already falls out of the comparison loop as false on its own.
…ackage codec

readZeroTerminated now uses Uint8Array.indexOf(0, offset) instead of a
hand-rolled scanning loop with its own bounds check: indexOf already
reports "not found" as a single -1 sentinel, so there is exactly one
place that decides whether the terminator was found.

asciiZeroTerminated now writes through a DataView instead of raw
indexed assignment: an out-of-range DataView offset throws, where a
plain bytes[index] = ... past the array's own end silently does
nothing, so a loop bound one iteration too long now fails loudly.

Test coverage is broadened to assert the exact thrown message and
error name for every OlePackageFormatError/OlePackageWriteError case,
plus new cases for the source-path/temp-path terminator and the
packaged-file size field's own truncation.
passwordToUtf16LeBytes now writes each UTF-16LE code unit via
DataView.setUint16 instead of two raw indexed byte assignments: an
out-of-range DataView offset throws, where a plain bytes[i] = ...
past the array's own end silently does nothing, so a loop bound one
iteration too long now fails loudly instead of leaving indistinguishable
output.
…ryptoAPI verifier

bytesEqual's sole call site always compares a SHA-1 digest (always 20
bytes) against a slice already fixed to that same verifier-hash
length, so a mismatched-length pair can never reach this private
helper; guarding against one was dead code for a case this module
cannot produce.

Added a test that shares one byte between the verifier's real SHA-1
and a tampered hash while complementing every other byte, which fails
only against a genuine byte-by-byte .every comparison, not one that
would accept any single matching byte.
passwordToAsciiBytes and createXorObfuscationArray now write through a
DataView instead of raw indexed assignment: an out-of-range DataView
offset throws, where a plain array[i] = ... past the array's own end
silently does nothing, so a loop bound one iteration too long now
fails loudly.

Test coverage now asserts the exact thrown message for the length and
codepoint guards, adds the Latin-1 boundary (U+00FF) and the emoji
surrogate-unit case, and adds a case where XORing a non-zero byte
against the array happens to produce zero, proving the
[MS-OFFCRYPTO] 2.3.7.6 zero-exception checks the transformed result
as well as the original byte.
… timeout

Per-test coverage instrumentation (Stryker's dry run, or plain
--coverage) measurably multiplies this test's own real cost well past
vitest's 5000ms default on a loaded machine, independent of anything
this suite is actually testing.
…e writes

encodeUnicodeStringValue now walks value.split("") instead of an
explicit charCodeAt loop, and drops the separate null-terminator write:
characterBytes is allocated one 16-bit unit longer than value itself
and starts zero-filled, so the reserved terminator slot already holds
the required 0. split("") walks by UTF-16 code unit (unlike spreading
a string, which walks by code point and would split a surrogate pair
across two array entries), matching [MS-OLEPS] 2.20's own definition
of Characters as 16-bit code units.

Every padding-only DataView write that stores the value 0 drops its
littleEndian argument, since 0 is byte-identical under either byte
order. The header's own CLSID write is removed entirely: GUID_NULL is
all zero bytes, exactly what streamBytes already holds fresh off
`new Uint8Array`.

Test coverage adds an assertion that the dictionary is written in
ascending PID order (not merely insertion order) and asserts the
exact thrown message for the VT_LPSTR write refusal, alongside the
existing VT_LPWSTR padding-boundary test.
…S wire tests

test-support/oleps.ts's own header writer drops the littleEndian
argument on writes that always store the value 0 (byte-identical
under either byte order), matching oleps/write.ts's own convention.

Two new test files add direct coverage the reader's own suite only
exercises in passing: oleps/wire.test.ts isolates writeGuid/readGuid
from write.ts and read.ts, whose only call sites overwrite the very
next field immediately afterward, silently absorbing a one-byte-over
write that a GUID-sized buffer now catches directly; test-support's
own oleps.test.ts covers propertySetStream's FMTID encoding, its
VT_I4/VT_FILETIME field builders, and its Characters-field padding.
readUint16LE/readUint32LE now delegate to DataView.getUint16/getUint32
instead of a hand-rolled undefined-checking loop: DataView's own reads
already throw a RangeError for an offset whose read would run past the
buffer's own end, so there is no separate bounds check to hand-write
and no separate error message to keep in sync with it.

Added direct test coverage for these readers and for
localFileHeaderNames/localHeaderCompressionMethod's own local-header
walk, including the exact boundary where headers run flush to the end
of the bytes and the case where a non-zero extra field must be
skipped to reach the next entry.
…LEPS writer

Every remaining view.setUint16(offset, 0) call in
encodeTypedPropertyValue and the stream header wrote a literal 0 into
a byte range new Uint8Array already zero-fills, and none of them is
ever overwritten afterward, so the write itself was a genuine no-op
matching the CLSID case already documented on the header.
… wrong slices

writeGuid computed each Data4 byte from digits.slice(start, end) where
start and end were two independently-written expressions sharing the
same loop index; a start shifted by a wrong index still ended at the
correct position, so the extracted substring merely gained extra
leading hex digits that setUint8's own mod-256 truncation silently
discarded, an equivalent transformation no input could ever surface.

hexByte now derives both ends of the two-character window from one
shared expression, so a wrong index always extracts a genuinely
different byte. The Data4 loop itself now walks a literal index array
rather than a comparison-bounded for loop, since a bound one iteration
too long previously landed on the byte immediately after the GUID,
which every real call site immediately overwrites with its own next
field regardless.
…quivalent mutant

The allocation reserves one extra, already-zero byte for the field's
own null terminator, so a loop bound one iteration too long wrote
NaN (from charCodeAt past the string's own end) through setUint8,
which coerces to 0 -- byte-identical to the terminator already there,
and genuinely undetectable by any test.

Walking value.split("") instead removes the comparison bound
entirely, so there is no off-by-one left to be silently absorbed by
the reserved terminator slot.
…tly testable

padMessage's own high-half write was byte-order-correct but every
test message is under 512 MiB, so high is always 0 and its own byte
order made no observable difference to any hash output.

writeBitLength64 pulls the pair of writes out of padMessage into its
own function taking a bit length directly, so its high half's little-
endian order is exercised against a fabricated >2^32 value without
hashing an actual 512 MiB buffer.
…gnature mismatch

The existing "fewer entries than the archive holds" case can never
distinguish a missing signature check: whether the loop exits via a
signature mismatch or by simply running past the end of the bytes,
the same final throw fires with the same message, since it names only
the requested entryIndex and nothing the loop itself observed.

The new case places 40 zero bytes -- long enough to read as a
well-formed but nonsensical header, without the local-file-header
magic -- right after one real entry, so a walk that skipped the
signature check would misread them as a second header and return
its own (zero) compression-method field instead of throwing.
nestZip wraps its innermost archive from the inside out, so
level-(MAX_WALK_DEPTH - 1).zip is the outermost entry and level-0.zip
is the one directly containing innermost.txt -- the opposite order
the existing assertion named, which the walk's own outermost-first
ancestor chain never actually produced.
…nd add gap coverage

Every PropertySetFormatError assertion now checks the exact thrown
message rather than just the error class, and a wrong-type case is
added for a date field alongside the existing string-field one.

New cases cover splitKeywords' own filter directly (a hand-written
KEYWORDS value with doubled/trailing delimiters, and one that is
entirely empty/whitespace segments), writing no KEYWORDS property for
a defined-but-empty array, an explicit empty string reading back as
absent, and a genuine VT_LPSTR property built through test-support's
own encoder (writePropertySetStream itself refuses to encode one, so
a round trip through this module's own writer can never reach
stringValue's VT_LPSTR branch).
…ard and redundant default case

Every requireBytes call site derives its offset and length from a
getUint32/getInt16 read (always non-negative) or a positive literal or
constant, so offset<0 and length<0 can never actually be true --
dead defensive code against an input this module never produces.

The property-type switch's own trailing default case restated exactly
what falling out of a switch with no matching case already does on
its own; its break was dead code every other case already carries,
and removing it is byte-identical behavior. The explanatory comment
moves above the switch instead of vanishing with the case.

decodeCodepage and truncateAtNull are pulled out as their own
functions and exported: neither one's own edge (the raw === 0
two's-complement boundary; a string with no null code unit at all)
is otherwise independently observable, since both are only ever
used through comparisons or fixtures that happen to mask the
difference.
…ughout the OLEPS reader

Every PropertySetFormatError assertion in this suite checked only the
error class, never its message -- so two different structural checks
throwing the same error class were indistinguishable, and a corrupted
string literal or a wrong arithmetic operator inside requireBytes'
own condition or message template survived unnoticed as long as some
PropertySetFormatError still fired somewhere in the call.

Every requireBytes call site (the stream header, the PropertySet
packet header and its own declared Size, the dictionary, the CodePage
property's TypedPropertyValue, a property's own TypedPropertyValue
header, each of VT_I2/VT_I4/VT_FILETIME's own value, and
CodePageString/UnicodeString's own Size-or-Length and Characters
fields) now gets its own boundary test asserting the exact thrown
message, truncated to exactly one byte short of what that specific
check requires.

Testing a check deeper than the packet's own declared-Size guard
needs truncatedPastDeclaredSize: corrupting the Size field down to 0
(nothing else in this reader reads it again) lets the array's own
physical truncation reach a boundary further in without the earlier
Size check intercepting it first, since it runs before every deeper
check in the same function.

Also adds direct tests for decodeCodepage and truncateAtNull (both
newly exported from read.ts), the CodePage property's own wrong-type
error, and an exact non-zero TypedPropertyValue padding message with
two bytes whose little-endian and big-endian readings genuinely
differ, proving the byte order rather than merely the zero check.
…g guard

toUpperCase() already leaves every one of the 2048 lone surrogate code
units (0xD800-0xDFFF) completely unchanged (verified directly against
every value in the range), since none of them has a case mapping of
its own -- the length-1 fallback already returns exactly the same
unit an explicit surrogate guard would, making it dead code.

compareEntryNames now walks left.split("") instead of a for loop
bound by left.length: since both strings are already known
equal-length at that point, an out-of-range comparison one iteration
too long compares charCodeAt(left.length) against itself on both
sides (NaN against NaN, by construction identical), an equivalent
mutant no input could distinguish.

deepestDepth and exceedsVersion3StreamCeiling are pulled out as their
own exported functions: deepestDepth's sole call site never observes
its own count === 0 branch (linkSiblings returns before reading the
argument at all for an empty sibling list), and
exceedsVersion3StreamCeiling's condition would otherwise need
constructing and writing an actual 2 GiB+ stream for every mutant of
the boundary itself, not merely the one real test that must still
exist for the thrown message's own exact text.
…case-mapping boundaries

Adds direct tests for deepestDepth and exceedsVersion3StreamCeiling
(both newly exported from write.ts), plus one real end-to-end test
proving the version-3 stream-size ceiling's own exact thrown message
against a genuine 2 GiB+1 byte stream.

Two name-ordering tests prove the simple (single-code-point) case
mapping [MS-CFB] 2.6.4 requires, not JS's own full mapping: the
Kelvin sign (U+212A) uppercases to itself but lowercases to plain
'k', giving opposite sort orders against 'L' under the two mappings;
'ß' uppercases to the two-character "SS" under the full mapping,
so the simple mapping's own fallback must leave it as itself (0xDF)
rather than taking the expansion's first character.

New mini-stream and FAT/mini-FAT/DIFAT region tests check what the
existing round-trip fixtures never observed directly: a zero-length
entry's own startSector field (readCompoundFile's own zero-size
shortcut never inspects it, so a round trip alone can't prove it),
sequential mini-sector allocation across more than one mini-resident
stream, the mini-FAT sector count crossing its own 128-mini-sector
boundary, the FAT's unused tail entries past a file's real total
sector count, and each chained DIFAT sector's own fatIndex/terminator
values -- all only reachable with a multi-FAT-sector, DIFAT-chained
fixture, never the minimal single-sector one.
… in test-support/oleps.ts

view.setUint16(2, 0) and view.setUint32(4, 0) wrote literal zeros into
a byte range new Uint8Array already zero-fills, matching the same
pattern already removed from oleps/write.ts's own header write --
only the littleEndian argument had been dropped here previously, not
the redundant write itself.
…-index fallbacks

Several arrays here (fatSectors, and the parallel bigStreamRecords/
bigSectorCounts and smallStreamRecords/miniChunks pairs) were indexed
by a shared loop counter bounded by the same array's own length, so
every ?./?? fallback for the array's out-of-range undefined was
provably unreachable -- noUncheckedIndexedAccess forcing a fallback
to exist for a case that could never actually occur.

bigEntries and miniEntries now pair each record with its own
already-computed sector count or padded chunk directly (one array of
paired objects rather than two same-length arrays walked in
lockstep), and fatSectors.forEach replaces its own indexed loop,
since fatSectors is already the identity array [0, 1, ..., count-1].
The directory-entry writer's own childId now narrows through
destructuring node.children[0] instead of a `?? node` fallback that
could only be reached after a sibling-count check had already proven
the array non-empty.

The header's own leading-magic-bytes write and 109-entry DIFAT array
now walk their arrays directly (forEach, and a literal-length index
array) rather than a comparison-bounded for loop: a bound one
iteration too long previously wrote into a byte range (the CLSID
field's first byte; the first FAT sector's own data) that was either
already zero or immediately overwritten by a later real write
regardless, an equivalent mutant no test could ever observe. The
mini-stream copy's own length guard is dropped for the same reason:
Uint8Array.prototype.set with a zero-length source is already a
no-op.
…dCompoundFile

New fixtures exercise readCompoundFile through shapes the existing
suite never reached: several mini-resident streams in the same mini
stream (each at its own sequential mini sector), several FAT-resident
streams, a storage with more than one sibling child, a directory
needing more than one 512-byte sector, a file needing more than one
FAT sector, a mini stream needing a second mini FAT sector, and the
directory-sector-count header field's own version 3 vs version 4
difference.

A new describe block covers compoundFile's own input validation
(empty/over-long/non-ASCII names, empty path segments, duplicate
paths), and one direct assertion checks the root entry's own
ENDOFCHAIN starting sector when there is no mini stream at all --
readCompoundFile's own zero-size shortcut never inspects that field,
so no round trip could otherwise prove it.
sector is always a chain's own start (a u32 header/entry read) or a
prior fatEntry return (itself a u32 read), so it can never actually be
negative at this call site -- a defensive check against an input this
closure never receives, matching the same dead guard already removed
from oleps/read.ts's requireBytes.
…structural checks

Every CompoundFileFormatError assertion in this suite previously
checked only the error class via a shared expectFormatError helper,
never the message -- so two different structural checks throwing the
same error class were indistinguishable from each other, and a
corrupted string literal or wrong comparison operator survived
unnoticed as long as some CompoundFileFormatError still fired.

New cases cover, each with its own exact thrown message: the
signature and header-length checks, major version (both directions
of the sector-shift-vs-version mismatch), byte order, mini sector
shift, mini stream cutoff (including the exact boundary at the mini
sector size itself), the header DIFAT array and DIFAT-chain sector
bounds (one past the file's own total), a DIFAT chain that cycles, no
FAT sectors at all, a FAT chain stepping outside the file or onto a
role-marker sector or cycling, an empty directory chain, a non-root
first entry, mini-FAT chain bounds/role-markers/cycles mirroring the
FAT chain's own, a stream's declared size exceeding its chain, the
directory tree linking outside its own entries or reaching an entry
twice, an out-of-range name length, an unsupported object type, a
second root-typed entry reached via the tree, and the cumulative
extraction budget.

The DIFAT-chain cases need a fixture test-support/cfb.ts's own
compoundFile cannot build (it never spills the DIFAT past the
header's 109-entry array), so those two reach for ../cfb/write.ts's
writeCompoundFile purely as a source of valid DIFAT-chained bytes to
corrupt -- not to test a round trip, which write.test.ts already
does.
…o-sector guard

Math.ceil(0 / fatEntriesPerSector) already evaluates to 0, so the
miniSectorCount === 0 special case in the ternary produced a value
byte-identical to the general formula. Collapsing it removes an
equivalent-mutant opportunity without changing behaviour.
…t membership

ids.length >= sectorCount (and the mini-FAT analogue) is a count-based
pigeonhole bound: it can only ever trip on a sector already visited,
whose own FAT entry is deterministic, so a count off by one merely
replays an earlier step before reaching the identical eventual outcome
-- unobservable through any thrown message. A direct visited-set
check, matching the directory tree's own cycle detection, fires on the
exact iteration a sector repeats instead.

Also removes the id >= entryCount guard in the directory tree walk:
entries is a dense array of exactly entryCount elements, so an
out-of-range id already reads back as undefined there regardless of
magnitude, making the explicit bounds check redundant.
…d DIFAT/size boundaries

Every writeCompoundFile input-validation test previously checked only
the thrown error's class via CompoundFileWriteError, never its
message, so a corrupted string literal in any of the four throw sites
survived unnoticed as long as some CompoundFileWriteError still fired.

Extracts the stream-size field's high 32-bit word into its own
highSizeWord helper, exported and tested directly against plain
numbers -- the same treatment exceedsVersion3StreamCeiling already
gets, and for the same reason: proving the division holds would
otherwise need constructing and writing an actual 4 GiB+ stream.

Adds boundary fixtures for the DIFAT-chaining decision (exactly 109
FAT sectors needs no chained DIFAT sector, 110 needs exactly one) and
bumps the shared FAT/mini-FAT/DIFAT-region fixture from 8 to 24 MiB so
its own DIFAT chain is at least three sectors long -- the per-sector
index and next-pointer arithmetic in that chain is otherwise
indistinguishable from a subtly wrong variant, since at sector 0 every
candidate formula agrees.

Removes the FAT- and DIFAT-sector role-marking loops' own hand-written
comparison bounds in favour of walking Array.from's bounded index
list: an off-by-one there would mark one sector past its own region,
but that sector is always the very first one the next region's own
chain-writing call writes right afterwards, making a stray extra
iteration unobservable in the finished file regardless. The directory
entry name-writing loop gets the same treatment, for the same reason
(the one extra byte pair it could write sits in a padding gap that is
already zero and never otherwise touched).

Adds a direct check that the FAT table's own entries for the FAT and
DIFAT sectors themselves read back as FATSECT/DIFSECT role markers,
which no reader ever inspects by following a chain, so nothing else
in this suite exercised it.
…xture builder

compoundFile's own internal decisions -- sibling-node reuse across
storage and stream nodes, exact directory-entry byte layout, loop
boundaries -- were previously exercised only indirectly through
../cfb/read.test.ts and ../cfb/write.test.ts reading back what it
writes. A read-back check cannot distinguish many of these: three
streams sharing a common storage prefix produce the same paths back
whether that storage is written once and shared or duplicated once
per stream, since path construction only concatenates prefix and
name.

Also removes two guards this builder's own callers can never trigger:
node.name.length === 0 in checkedName (every name reaching it was
already proven non-empty, including the root's, which now carries its
real "Root Entry" name from creation instead of a separately-typed
placeholder overridden just before the one call site that used to
need it) and leaf === undefined in compoundFile's own path-segment
validation (String.prototype.split never returns an empty array, so
.pop() on it can never actually be undefined; folded into the
existing leaf.length === 0 check via a single falsy test that also
narrows leaf's type).

Adds a name its() error class check for PropertySetFormatError, the
same class-identity check every other named error in this package
already carries.
…record()'s own return values

Linking each storage's children by looking their records back up in
recordOf afterwards needed a childRecord !== undefined guard for a
case that can never occur: record() already visits every node
reachable from root, including all of node's own children, before the
sibling-linking pass runs, so the lookup always finds an entry. Using
each recursive call's own return value directly removes the lookup
(and its guard) entirely -- only the true last sibling's own "next"
read can be the array's out-of-range undefined, which is exactly the
existing NOSTREAM fallback already for.

The same reasoning simplifies a directory entry's own first-child
pointer: firstChild && recordOf.get(firstChild)?.id short-circuits to
undefined without a lookup at all when there is no child, narrowing
firstChild for the lookup on the other side -- replacing && with ||
now produces a wrong id the moment there is a real first child,
where the previous === undefined ternary's two branches carried no
such difference (recordOf.get's own fallback already produced the
same NOSTREAM either way).

Removes the header DIFAT array's own i < fatSectors.length ternary for
the same reason as a nearby comment already documents about its loop
bound: fatSectors[i] is already undefined past the array's own length,
and ?? FREESECT already turns that into the identical padding value
the ternary's false branch spelled out a second time.

Strengthens the mini-FAT tests: filler and boundary-straddling stream
payloads are now distinct per stream (not uniformly zero, and not
uniformly a single-mini-sector ENDOFCHAIN chain) so a mini-FAT sector
written to the wrong physical location, or read from the wrong offset
within the combined mini-FAT buffer, actually corrupts observable
bytes instead of silently duplicating content that happened to already
match.
…eout

v8 coverage instrumentation multiplies this test's real cost well past
the default 5000ms budget: a 300 KiB byte-fill loop plus a full round
trip runs in well under a second uninstrumented, but has timed out on
a loaded GitHub runner under coverage in CI. A fixed, generous timeout
removes the flake without shrinking the payload the fixed-point
FAT-sector-count loop needs to actually grow past 1.
…t mutant coverage

A fixture computed at a describe block's own top level runs during
Vitest's collection pass, before any test executes -- module-load-time
code Stryker's own coverage analysis treats as "static" and cannot
attribute to a specific test. The DIFAT-chaining arithmetic this
24 MiB fixture exists to exercise is reachable nowhere else in this
suite, so every mutant reachable only through it got no per-test
coverage at all and fell back to Stryker's "run everything" path,
which silently failed to catch several genuine regressions introduced
purely by this fixture's own top-level placement. Building it in
beforeAll instead runs it within the describe block's own test-
execution phase, where coverage analysis attributes it correctly.

Removes another genuinely equivalent condition in the same fixed-
point loop: HEADER_DIFAT_ENTRIES (109) is smaller than
difatEntriesPerSector (127 or 1023, depending on sector size) for
every version this writer supports, so whenever neededFat is at or
under 109 the ceil-of-a-small-negative-number formula the ternary's
own false branch already computes evaluates to 0 regardless -- the
same value the true branch spelled out a second time. Math.max(0, ...)
replaces it, making that invariant explicit instead of leaving it to
depend on the relationship between two separate magic numbers.

Adds a nameLength of exactly 0 to read.ts's boundary coverage: the
only value that is both under the 2-byte minimum and even, needed to
prove that specific disjunct matters on its own (every odd value below
2, such as 1, is already caught by the separate oddness check).

Adds direct FAT/mini-FAT padding-tail and role-marker checks to
test-support/cfb.ts's own suite, mirroring the equivalent checks
../cfb/write.test.ts already has for the real writer: the chain()
helper's own loop bound, and the FAT-sector role-marking loop, were
genuinely unobserved by every existing indirect (read-back) test.
…hared beforeAll

Stryker's per-test coverage analysis only attributes code executed
inside an it() body to that specific test -- a beforeAll hook runs
outside every individual test's own tracked window, confirmed directly
against a live mutation run (the DIFAT fixture's own tests carried
zero entries in any mutant's coveredBy list once built there). The
DIFAT-chaining arithmetic this 24 MiB fixture exists to exercise is
reachable nowhere else in the suite, so every mutant reachable only
through it fell back to whatever coverage its hybrid static/runtime
classification happened to carry from unrelated tests elsewhere,
letting genuine regressions in that arithmetic go uncaught.

Extracting a small bigDifatFixture() factory called at the top of each
test, rather than a shared describe-level value, costs a fraction of a
second per test to recompute and buys correct per-test attribution.
… directly

Deriving a directory entry's first-child pointer by re-reading
node.children[0] back out of a node -> record map, after record()
already built that same information once while assigning ids, needed
an optional-chained lookup whose absence case can never actually
happen: every node reachable from root, including all of node's own
children, is recorded before the map is ever read from. Storing the
child link on each DirectoryRecord directly, the same way rightId
already is, removes the redundant map (recordOf had no other reader
left once this changed) and the lookup along with it.

Removes another write of a value already guaranteed by the buffer's
own zero allocation: this test-support builder's own stream sizes
never approach 2^32, so the stream-size field's own high 32-bit word
was always the zero the buffer already held before this line ran.

Adds a direct byte-level check that the root directory entry's own
name decodes to "Root Entry": nothing functional depends on it (as
../cfb/read.ts's own comment already says), so only reading it back
off the raw bytes can tell a real name from an empty placeholder.
…ergence check

fatSectorCount and difatSectorCount's fixed-point loop only ever assigns
difatSectorCount from neededDifat computed the same round from neededFat,
so difatSectorCount === g(fatSectorCount) is an invariant the loop
maintains from its first iteration onward. Once neededFat matches
fatSectorCount, neededDifat is g(neededFat) === g(fatSectorCount), which
the invariant already guarantees equals difatSectorCount -- the second
half of the break condition could never once observe a mismatch the
first half didn't already rule out.
…ded index list

An off-by-one on this loop's own comparison would run one slot past
difatEntriesPerSector, writing into the exact byte offset the
unconditional terminator write immediately below writes to next for
that same sector -- so a stray extra iteration here is always
overwritten before the sector is ever read back, regardless of
whether the extra iteration's own write even happens. Walking
Array.from's own bounded index list removes the comparison, and the
mutation opportunity along with it.
… not once per describe block

A shared const built once at describe-setup time runs before any
individual test, so Stryker's per-test coverage tracker cannot
attribute a mutation in writeCompoundFile's own body to whichever
assertion in this block would actually catch it -- every mutant
reachable only through that shared construction was misreported as
surviving regardless of whether a real test killed it. Rebuilding the
fixture fresh inside each it() gives every assertion its own tracked
call to the code it exercises.
…ields stay untouched

Two writer regions had no assertion proving they hold their expected
zero/FREESECT value rather than whatever a bug happened to leave
there: a DIFAT sector's own trailing slots past the last real FAT
sector reference (only ever set by the region's initial FREESECT
fill, never explicitly written), and an unallocated directory entry's
own size fields (never written at all, so they must still read the
zero the buffer's allocation started with). Both are now checked
directly.
Every valid mutant across archive-codec is now genuinely killed by a real test,
with no Stryker disable comments anywhere in the package, so the gate is the
literal maximum rather than a derived-with-slack figure.
@Mearman
Mearman force-pushed the feat/100-percent-mutation-archive-codec branch from d9bd356 to 147cf7c Compare September 13, 2026 12:22
@Mearman
Mearman marked this pull request as ready for review September 13, 2026 12:36
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review Completed 2026-09-13T12:49:13.180471Z 147cf7c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Mearman
Mearman merged commit 903f376 into main Sep 13, 2026
26 checks passed
@Mearman
Mearman deleted the feat/100-percent-mutation-archive-codec branch September 13, 2026 12:49
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.11.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant