Modern API 1.0: full IMAP4rev1/rev2 client with per-requirement compliance measurement (M0–M6)

[LoveAndCoding]

What this is

The complete modern-API implementation, M0 through M6, ending at version 1.0.0: a spec-compliant, fully typed IMAP client with a promise/async-iterable API, measured throughout by the repo's per-RFC-requirement compliance suite. Every milestone ratcheted the same matrix per-row (status + failure kind), with regressions blocked at every merge; every milestone's snapshot and close notes live in docs/compliance-history/.

The M0–M6 arc

• M0 — violations burn-down (notes): the pre-existing suite measured 118 violation row-profiles against the old code. M0 fixed the connection/security families (TLS identity per RFC 9525, STARTTLS sequencing, greeting policy, parser case-insensitivity and tolerance, bigint number64) with no new public surface: 118 → 3 violations.
• M1 — client shell and auth (notes): ImapClient, typed config, the SASL framework (PLAIN/OAUTHBEARER/XOAUTH2/LOGIN-fallback), ENABLE, capability epochs, the typed error hierarchy, and removal of the legacy Session class. 486 pass.
• M2 — mailbox management (notes): list/lsub (LIST-EXTENDED, LIST-STATUS, SPECIAL-USE), select/examine → MailboxSession, create/rename/delete/subscribe, STATUS, NAMESPACE, APPEND, the mUTF-7 codec. 602 pass.
• M3 — message operations (notes): literal streaming, the FETCH engine (async-iterable, part streaming), the typed SEARCH criteria compiler + ESEARCH, STORE verbs, COPY/MOVE + UIDPLUS, EXPUNGE, MULTIAPPEND/CATENATE, SequenceSet, and the seq facet mirroring every UID-grain verb. 780 pass.
• M4 — live mail and synchronization (notes): IDLE + the updates() iterator, CONDSTORE/QRESYNC, NOTIFY, SORT/THREAD (+ SORT=DISPLAY, ESORT), SEARCHRES, WITHIN, FUZZY. 882 pass.
• M5 — extension families (notes): SCRAM-SHA-1/256 (with a terminal-failure MITM protection) + ANONYMOUS, the QUOTA/ACL/METADATA/URLAUTH facets, COMPRESS=DEFLATE, UNAUTHENTICATE, UIDONLY, CONVERT, REPLACE, LANGUAGE/COMPARATOR, referrals, UTF8=ACCEPT/ONLY, X-GM-EXT-1, the CONTEXT updating machinery, and RFC 9394 PARTIAL. Its phase review caught and fixed a critical queue deadlock (compress()/unauthenticate() racing an in-flight command). 1138 pass.
• M6 — close-out and 1.0 (notes): SHOULD/MAY sweep (zero unexplained rows; 11 satisfied-by-mechanism ledger entries), all five tracked follow-ups resolved with zero deferrals (including closing the pre-TLS/pre-codec complete-line injection window at all three topology switch points), typedoc (1029 warnings → 0), docs/MIGRATION.md, README rewrite, CHANGELOG, version 1.0.0, and a three-lens final review (zero criticals).

Compliance: before → after

Measure	Baseline (pre-M0)	Final (1.0.0, M6 snapshot)
Violations	118 row-profiles (73 requirements)	6 — every one carries a written adjudication
Passing requirement-profiles	— (332 at M0 close)	1138
Unimplemented	819 (at M0 close)	2 — both adjudicated (deliberate scope boundary)
Untestable (cataloged rationale each)	427	451
MUST/MUST NOT (testable, both profiles)	—	927/929 raw (99.8%); 100% excluding the 2 adjudicated rows
SHOULD	—	78/84 raw (92.9%); 100% excluding the 6 adjudicated rows
MAY	—	133/133 (100%)
Suite problems	—	[], nine consecutive byte-identical full runs
Unit tests	232 (1 failing)	1936 (all passing, 133 files)

Adjudications (read this before reading "violations: 6" as defects)

All non-passing rows — the 6 violations and 2 unimplemented — are documented, deliberate decisions in docs/compliance-adjudications.md (~25 entries):

• RFC9051-7.1-1 (SHOULD, rev2): pre-confidentiality ALERT content is surfaced with an untrusted marking rather than suppressed — trading one SHOULD for two MUSTs.
• RFC9051-A-1 (MUST, rev2): the client does not auto-ENABLE IMAP4rev2 — a deliberate profile-stability decision (rev2 behavior differences are opt-in).
• RFC7162-3.1.3-5/-6 (SHOULD, both profiles): no automatic CONDSTORE re-probe/retry — callers own retry policy.
• RFC9051-2.3.2-1/-2 (rev2): no automatic $Junk/$NotJunk conflict STOREs and no refusal of $Forwarded removal — keyword policy belongs to callers; the library exposes the raw signal.
• RFC 4422 security-layer block + SCRAM channel-binding block (22 row-profiles, untestable): vacuous-by-design — no shipped mechanism negotiates a SASL security layer (TLS is the security layer), and SCRAM-*-PLUS is a documented non-goal with reactivation conditions written into the catalog.

Breaking changes vs 0.9

The public API is new — 0.9 was a TypeScript port of node-imap. Session is gone, callbacks are gone, event-DSL fetch/search are gone, boolean tls config is gone. docs/MIGRATION.md maps every node-imap idiom to its 1.0 equivalent (all destination samples are compile-checked in CI via npm run typecheck), including the one data-model change (getBoxes() tree → flat list()).

Deferred, with rationale

• Auto-reconnect, TRYCREATE auto-create, MIME body decoding, connection pooling, client-side seq↔uid mapping: design non-goals (spec §13) — the primitives to build them are exposed.
• SCRAM-*-PLUS channel binding (tls-exporter per RFC 9266): legitimate post-1.0 feature; catalog rows carry reactivation conditions.
• Post-1.0 hardening notes (M6.8 review): a second-layer discard for the compressed-UNAUTHENTICATE boundary guard; root-exporting four updates()-signature types.
• npm run test:e2e (real-server suite) was not run in this environment (no credentials); all other gates ran on every merge.

Verification

npm test (1936), npm run typecheck (incl. compiled doc samples), npm run typecheck:compliance, npx eslint . (0 errors), npm run docs (0 warnings), npm run build + npm pack --dry-run (tarball: 344 files / 2.2 MB), and npm run test:compliance — nine consecutive byte-identical runs across all 1597 requirement-profile rows, problems: [].

---

Per the session's standing decision this PR is opened for human review and is not merged by the automation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Jvj25KC7SRUFHsyTzswqsW

---

Generated by Claude Code

[LoveAndCoding]

Review Report — "Modern API 1.0" (M0–M6, v1.0.0)

Recommendation: HOLD FOR CHANGES

This is impressively thorough, well-documented work, and the compliance-measurement discipline is real. But the review surfaced three Critical and six High findings that are merge-blocking, two of which were reproduced as live defects during validation:

• Two remote, one-line process crashes: a malformed tagged or untagged server response throws a raw exception that no listener catches, taking down the whole Node process — reproduced end-to-end (exit 1) against the real parser pipeline. For a network client that parses untrusted server bytes, this is a denial-of-service in the library's own stated "never crash" (I-6) invariant.
• A SCRAM mutual-authentication fail-open: the "terminal-failure MITM protection" the PR headlines can be bypassed by a server/MITM that never presents a verifiable server-signature; the client accepts the session as mutually authenticated.
• Plus a silent connection wedge, an unbounded hang, an auth-breaking reused-instance bug, and a logout() that silently no-ops after reconnect (leaving a live, authenticated connection the caller believes is torn down).

None of these are subjective. Each has a concrete file/line and, for the reproduced ones, a trigger. They should be fixed (and regression-tested) before merge. The large Medium tier is mostly hardening and wire-correctness edge cases that can be follow-ups, but several (injection surfaces, capability-epoch staleness) deserve a look given this is a 1.0 security-sensitive library.

Validated finding counts: 3 Critical · 6 High · 26 Medium · 12 Low. (3 candidate findings dropped as invalid after validation; 2 reclassified as not-relevant dead code.)

---

Critical

1. SCRAM server-signature can go unverified (MITM fail-open)

ScramMechanism.finish() (src/sasl/scram.ts:351-360) takes zero parameters while its interface declares finish(data, ctx) (src/sasl/mechanism.ts:105); TS bivariance hides the mismatch. AuthenticateCommand.accept() always calls finish(null, ctx) (src/commands/authenticate.ts:161-163). Combined with handleServerFinal's "case 1" (scram.ts:41-48) resolving normally when no server-final message was ever presented, a server/MITM that never delivers a verifiable v= through the one channel this code inspects is accepted as mutually authenticated. Directly undercuts the PR's headline MITM protection claim.
Fix: Have finish(data, ctx) parse and verify v= via timingSafeEqual whenever a server-final wasn't already consumed; make a tagged-OK "success" with no verifiable signature fail closed. Also update authenticate.ts to pass the real data.

2. Malformed tagged response crashes the whole process

TaggedResponse (src/parser/structure/tagged.ts:30) calls StatusResponse.match(tokens,2) with no try/catch, unlike the untagged path. APPENDUID/COPYUID (RFC 4315) ride the tagged OK, so their parse-throw sites are primarily reached unprotected. Reproduced a real process exit(1) feeding A1 OK [UIDVALIDITY 0] / malformed [APPENDUID …] through the real pipeline.
Fix: Wrap TaggedResponse's StatusResponse.match() in try/catch with an UnknownContent fallback; also fix #4 below.

3. Trivial malformed untagged line crashes the process

UntaggedResponse's constructor reads tokens[1] (src/parser/structure/untagged.ts:85) and contentTokens[0] (:97) without null-guards, before its own try/catch loop. Reproduced TypeError-to-process-crash from the one-line inputs "*\r\n" and "* \r\n", in the exact file meant to be the tolerance backstop.
Fix: Guard both accesses (!secondToken || !secondToken.isType(...), and check contentTypeToken) → ParsingError/UnknownContent instead of TypeError.

---

High

4. Parser pipeline has no 'error' listener (crash amplifier)

connection.ts resetProcessingPipeline() (src/connection/connection.ts:782-830) attaches untagged/tagged/continue/unknown listeners to this.parser but never 'error'; no process.on("uncaughtException") exists anywhere in src. .pipe() does not forward destination errors. This is the linchpin turning #2/#3 (and any future parser error) into full process crashes.
Fix: Attach an 'error' listener to the parser (and lexer/pipeline) that converts a stream error into a controlled connection-level failure.

5. Compression codec errors silently wedge the connection

compress() routes zlib inflate/deflate errors to onSocketError (src/connection/connection.ts:1209-1211), which only emits connectionError — no socket destroy, queue stop, or state reset. A codec error fires on the transform, not the socket, so no teardown runs. connected stays true, reads are dead, writeBytes keeps feeding a destroyed deflate — a permanent silent wedge from one malformed compressed frame.
Fix: Route wrapCompression's onError through a real teardown (destroy socket + stop queue), distinguishing codec from socket errors.

6. No timeout on post-greeting negotiations → indefinite hang

execute-command.ts has no timeout mechanism at all. The post-greeting isolated-command awaits — STARTTLS (connection.ts:997/1032/1104), COMPRESS (:1172), UNAUTHENTICATE (:1272) — and runCommand() generally are unbounded; only TCP-connect/greeting/TLS-handshake are timed. A server that accepts the connection then goes silent (or a graceful FIN during the STARTTLS window) hangs connect()/the caller forever and wedges the held queue — contradicting the file's own "never hangs" claim.
Fix: Bound these negotiations with a command timeout that tears down + rejects on expiry; extend teardown detection to graceful close throughout the connect() ritual.

7. Command submitted after CommandQueue.stop() never settles

AsyncQueueContext.add() (src/connection/queue.ts:157-169) only dispatches if (this.running) with no rejecting else-branch. After stop(), a subsequent add() parks the command, never settled; because the queue is reused across reconnects, a later start() can dispatch that stale command against a fresh connection. unauthenticate() also lacks the !this.socket guard that starttls()/compress() have.
Fix: Reject synchronously in add() when stopped/torn down; add the missing unauthenticate() not-connected guard.

8. Reused SASL mechanism instances retain stale per-attempt state

mechanism.ts's contract (62-72) requires implementations usable as a literal mechanisms object to reset ALL per-attempt state in start(). Violated across the board: ScramMechanism.start() never resets verificationFailure; CramMd5Mechanism/OAuthBearerMechanism/XOAuth2Mechanism never reset stepCalled/errorPayload. On a reused instance, one transient failure permanently poisons the mechanism, and any finish() throw is force-classified terminal:true, blocking all fallback — turning a benign reconnect into unrecoverable auth failure.
Fix: Reset all per-attempt fields at the top of each start(); correct the false "ScramMechanism follows this rule" contract claim in mechanism.ts:71.

9. logout() silently no-ops after reconnect

logout() (src/client/client.ts:612-621) caches _logoutPromise but never clears it (unlike _unauthenticatePromise). Reconnect on the same instance is explicitly supported, so connect→logout→connect→logout returns the stale resolved promise from cycle 1 on the second logout — no LogoutCommand, no disconnect(), no state transition. The caller believes the session was torn down while the connection remains fully live and authenticated.
Fix: Clear _logoutPromise in the disconnected bridge; add a logout→connect→logout regression test.

---

Medium (26 findings — highlights)

• Implicit-TLS connect() re-entrancy gap — two un-awaited connect()s on imaps:// can leak/cross-wire a TLS socket.
• Stale transient-error listener across STARTTLS swap, never detached from the pre-upgrade socket.
• COMPRESS/UNAUTHENTICATE boundary-injection defense has an unproven split-write smuggling risk across the async inflate boundary.
• Post-negotiation topology surgery (STARTTLS/COMPRESS/UNAUTHENTICATE) not guarded before commandQueue.release() — a sync throw permanently wedges the queue.
• disconnect() is async but resolves before cleanup actually completes.
• Connection-local capabilityRegistry never invalidated on COMPRESS/UNAUTHENTICATE for Layer-1-only callers.
• Unbounded PBKDF2 iteration count in SCRAM blocks the whole event loop (server-controlled i=).
• OAUTHBEARER/XOAUTH2 don't reject \x01 in user/host/token fields — kvpair-separator injection risk if any field carries untrusted input.
• OAUTHBEARER/XOAUTH2 structured error diagnostics are dead code against a spec-compliant server (only fire on tagged OK; real failures end in tagged NO).
• authenticate()/logout() race leaks a raw internal IllegalStateTransitionError instead of the public error hierarchy.
• Public connection escape hatch bypasses every client-side safety gate, including the cleartext-credential policy — client.connection.runCommand(new LoginCommand(...)) sends a password in cleartext even with allowInsecureAuth: false.
• STARTTLS doesn't invalidate the client-level capability registry — violates the PR's own "capability epochs" invariant; a greeting [CAPABILITY] can fire capabilitiesChanged with pre-TLS (forgeable) data before STARTTLS runs.
• Unbounded UID expansion from VANISHED (EARLIER) 1:4294967295 — a one-line DoS via expandUidSet's unbounded loop.
• LOGIN fallback can't be excluded via an explicit mechanisms list — a SASL-only intent can still send the real password via LOGIN.
• CommandWriter's documented atomic-rollback guarantee is broken (chunks reassignment vs. length-truncation restore) — reproduced data loss, though latent (no shipped command currently triggers it).
• listMailbox() regresses a previously-fixed &-escaping bug that mailbox() already has — reproduced: listMailbox("Sent&Received") emits a raw, unescaped &, corrupting LIST/LSUB patterns on ordinary input.
• Empty SEARCH or/fuzzy operand breaks OR/FUZZY wire arity — reproduced: {or:[{},{subject:"x"}], from:"y"} silently compiles to OR SUBJECT x FROM y, changing AND semantics to OR.
• updates() iterator listener leak on a specific driver-acquisition-reject path in MailboxSession.
• NUL byte and DEL (0x7F) serialized without proper escaping in several CommandWriter paths.
• date()/dateTime() don't bound-check the year, producing malformed wire dates for out-of-range Date values.
• KEYWORD/UNKEYWORD SEARCH criteria emitted via astring instead of atom, silently quoting invalid keywords instead of throwing.
• Binary APPEND (literal8/RFC 3516) sent without checking the server advertised BINARY, unlike every other optional extension in that file.
• zlib.createInflateRaw() has no output-size cap — a decompression-bomb DoS vector from a malicious/compromised server.

Low (12 findings)

Mostly hardening, documentation-accuracy, and spec-hygiene items — including a contained (non-crashing) TypeError in ENVELOPE parsing, dead/unreachable defensive code (assertIn(), src/connection/utils.ts), unsanitized server text embedded in thrown error messages, an ALERT-trust-marking bypass via a stray space in the resp-code bracket, and several RFC-citation/documentation-accuracy nits.

---

Notably invalidated during independent verification (flagged for transparency, not for action)

• A suspected mUTF-7 decoder crash — the utf7 package never throws on malformed input; not a real issue.
• A suspected SequenceSet bug where "5:*,10:*" coalescing to "5:*" would drop messages — disproven; the coalescing is provably RFC 3501 §9-correct for any actual mailbox max.
• A suspected HIGHESTMODSEQ=0 rejection bug — disproven; conflated two distinct response-code grammars (the resp-code position correctly requires ≥1; the STATUS data-item position, which allows 0, is handled correctly elsewhere).

Scope notes

Deep-reviewed: all of src/ across 16 file groups using state/correctness/error-handling/security/spec-compliance/api-compatibility/fidelity/maintainability lenses as fit each group, including both mega-files (client/client.ts, client/mailbox.ts) read in full. Not independently re-audited: the generated docs/compliance-history/ snapshots, and the ~4MB test corpus's assertion strength/coverage (the "1936 passing tests" claim was not re-verified). RFC text could not be fetched directly in this environment (network-restricted); spec claims were checked against this repo's own verbatim-quoting compliance catalog plus established ABNF.

---

Automated review orchestration — findings independently re-verified against the running code where feasible, including live reproductions for the process-crash, SCRAM, and several writer/search findings.

---

Generated by Claude Code

[LoveAndCoding]

Review addressed — all Critical & High findings fixed, plus the Medium/Low tier

Thank you for the exceptionally thorough review — the reproductions especially made these fast to confirm and fix. Every finding was independently verified against the code before touching it (I did not treat any as automatic), each real fix landed with a dedicated unit test and an undo→red→redo→green revert-verification, and every push kept the full gate suite green (npm test, npm run typecheck, eslint 0 errors, and the compliance matrix byte-identical at 1138 pass / 6 adjudicated violations / 2 adjudicated-unimplemented / 451 untestable / problems: []). Unit tests grew 1936 → 2053.

Four commits, grouped by concern:

• 212e3a3 command-writer + search wire-correctness
• 9bd8dd6 parser crash-safety
• 6eaeafb SASL/auth correctness
• 147ce7e connection & client lifecycle hardening

Critical

#	Finding	Resolution
1	SCRAM mutual-auth fail-open	Fixed. finish(data, ctx) now verifies v= (timingSafeEqual) and fails closed unless a matching server signature was consumed — a server/MITM that jumps to a tagged OK with no verifiable v= is rejected. The old code's RFC5802-5.1-14 justification only excuses omitting server-final on failure (a tagged-NO path that never reaches finish()). Terminal-AuthError-on-forged-signature preserved.
2	Malformed tagged response crashes process	Fixed. TaggedResponse wraps StatusResponse.match() in try/catch; a bad resp-code riding a tagged OK ([UIDVALIDITY 0], malformed [APPENDUID]) preserves tag+status so the command still settles, and a following response still parses.
3	Malformed untagged line crashes process	Fixed. UntaggedResponse guards its secondToken/contentTypeToken access; "*\r\n" / "* \r\n" now yield ParsingError / the UnknownContent tolerance backstop (I-6) instead of a raw TypeError.

High

#	Finding	Resolution
4	Parser pipeline has no 'error' listener	Fixed. onPipelineError wired to parser/lexer/pipeline 'error' events → controlled connection teardown. Reverting reproduced the uncaught process-level exception.
5	Compression codec errors silently wedge	Fixed. Codec onError now forces a real teardown via the socket close cascade (destroy + queue stop + reset).
6	No timeout on post-greeting negotiations	Fixed. ImapClientTimeouts.command is now actually wired through (it was dead config) and bounds STARTTLS/COMPRESS/UNAUTHENTICATE + the CAPABILITY round trips; a silent server no longer hangs connect(). (Instant graceful-FIN reaction during the ritual I judged covered by the timeout bound rather than a larger socket-lifecycle rewire — flagged as a follow-up rather than pretending it's a full FIN-detector.)
7	Command after stop() never settles	Fixed. CommandQueue gained a stopped flag; add-after-stop rejects synchronously (and can't dispatch a stale command against a reconnected socket). unauthenticate() got the missing !this.socket guard.
8	Reused SASL instances retain stale state	Fixed. All four reusable mechanisms reset per-attempt state at the top of start(); the false "ScramMechanism follows this" claim corrected.
9	logout() no-ops after reconnect	Fixed. _logoutPromise cleared in the disconnected bridge; connect→logout→connect→logout regression test added.

Medium — all addressed

Fixed: implicit-TLS connect() re-entrancy guard · stale STARTTLS-swap error listener detached · boundary-injection split-write hardening (postBoundaryDiscard, cleared via a single chokepoint so a mid-swap throw can't strand it) · topology-surgery-throw guards that always release the queue · disconnect() now awaits real socket close · capability-epoch invalidation on COMPRESS/UNAUTHENTICATE (connection-local) and on STARTTLS success (client-level, via a new secureUpgrade event) — closing the I-2 gap · unbounded PBKDF2 iteration cap (1M) · OAUTHBEARER/XOAUTH2 reject \x01/CR/LF in fields · OAUTHBEARER/XOAUTH2 diagnostics wired to the real tagged-NO path · authenticate()/logout() race maps to a typed StateError · STARTTLS capability-registry invalidation (same secureUpgrade fix) · VANISHED (EARLIER) 1:4294967295 DoS capped (was 10.6s / massive memory) · explicit non-empty mechanisms list without LOGIN now excludes the LOGIN password fallback · CommandWriter atomic rollback fixed (captures the array reference, truncates) · listMailbox() &→&- escaping restored · empty SEARCH or/fuzzy operand now throws instead of silently changing AND→OR · updates() iterator listener leak on the driver-acquisition-reject path · NUL/DEL escaping in the writer · date()/dateTime() year bounds · KEYWORD/UNKEYWORD emit as atom not astring · binary APPEND (and REPLACE) gated on BINARY/IMAP4rev2 · zlib.createInflateRaw() output-size cap (256 MiB).

One judgment call — the public connection escape-hatch cleartext bypass: I did not add a functional guard. connection.runCommand()/LoginCommand have no path to transport-security state without a CommandWriter-level security-context plumb-through, which is architecturally significant, and Connection is documented as deliberately policy-agnostic (the Layer-1 escape hatch). I strengthened the doc comments on both to spell out the gap and rationale, and I'm flagging the plumb-through as a deliberate follow-up for your call rather than making a sweeping change unbidden. Happy to implement it if you'd prefer it in this PR.

Low — addressed

Fixed: contained ENVELOPE TypeError (now typed ParsingError) · ALERT trust-marking stray-space bypass ([ ALERT]) tightened · two throw-site server-text sanitizations (BYE-greeting rejection, LOGIN AuthError) escaping control bytes. Disproven: the assertIn() "dead code" finding — it lives in src/client/state.ts (not connection/utils.ts), and is actively used and tested. RFC-citation nits swept where spotted.

Notes on your three invalidated candidates

Agreed on all three (mUTF-7 decoder, SequenceSet 5:*,10:* coalescing, HIGHESTMODSEQ=0) — my re-check reached the same conclusions.

Deferred, explicitly (not silently)

• Escape-hatch credential plumb-through (above).
• Instant graceful-FIN reaction during connect() (timeout bound covers the hang; a live FIN-detector is a larger change).

Compliance stayed byte-identical throughout, so the fixes tighten client-side refusal/robustness on malformed-or-adversarial inputs the suite's fixtures don't exercise in those exact shapes, without moving any scored row. Ready for another look whenever you are — and let me know on the escape-hatch item.

---

Generated by Claude Code

[LoveAndCoding]

Automated multi-agent merge-readiness review — PR #18 (Modern API 1.0)

Recommendation: HOLD for changes (address the Critical + the untrusted-server-data robustness items), then merge-with-follow-ups

This is a large, unusually well-engineered and heavily self-documented 1.0. The compliance adjudications and migration/README docs were checked against the actual code and hold up (see G26 below) — the "6 violations" are genuinely deliberate, documented decisions, not defects. However, the review surfaced one Critical and several High-severity robustness defects, concentrated in two areas: (1) the response parser's handling of malformed/adversarial server data, and (2) connection/client teardown races. These are exactly the areas a spec-compliant client must get right because the server is an untrusted peer. None are in the adjudication ledger, so they read as genuine gaps rather than accepted trade-offs.

Confidence: Moderate. Because of a tooling limitation in this environment, the sub-agent review-validator second-pass could not run; findings below are single-reviewer deep-pass results, self-verified by direct code tracing (several token-by-token) but not independently validated. Several carry explicit reachability caveats, called out inline. The Critical in particular should be reproduced with a test before acting, but the trace is concrete.

Coverage (transparency)

27 file-groups were dispatched across ~154 src/ files + docs + a test-quality sample. 17 groups returned complete reviews (client.ts ×2, mailbox.ts part 1, client fetch/idle/state, command infra search/fetch-store/auth-verb/acl-quota, parser core/fetch/mailbox, connection.ts ×2, connection queue, protocol, sasl, docs). 1 group returned a partial (G12). 9 groups did not return findings in time (forced-async pipeline stalls): mailbox.ts parts 2-3, client config/facets, commands base/writer/collector, select/status/list, misc mailbox commands, parser misc (incl. text.code.ts), lexer + top-level, and the test-quality sample. Note that several gap files (e.g. writer.ts, collector.ts, base.ts, mailbox.ts) were read as dependencies by adjacent groups, so they received partial indirect coverage. The 3047-line mailbox.ts and the lexer were reviewed only in part — a re-run is advisable for those before merge.

---

Findings (most-severe first; single-reviewer, validator pass unavailable)

CRITICAL

C1. Nested-multipart BODYSTRUCTURE mis-parsed → uncaught TypeError on common real-world mail — src/parser/structure/fetch/body.structure.ts:326-355 (child loop at 332-335). MessageBodyMultipartStructure builds every child via new MessageBodyStructure(...) without the multipart-vs-single-part check that already exists a few lines up (the MESSAGE/RFC822 branch, lines 250-267). For a multipart/* whose child is itself multipart/* (i.e. virtually every HTML+plaintext-with-attachment email, and all signed/encrypted mail), the split yields 2 blocks instead of 7 and destructuring throws a raw TypeError rather than parsing. This would break FETCH BODYSTRUCTURE/BODY for a large fraction of real messages. Fix: apply the existing multipart-detection pattern recursively in the children loop; add a nested-multipart regression test. (Traced token-by-token; reachability depends on real server responses but nested multipart is ubiquitous.)

HIGH

H1. Teardown racing an in-flight connect() hangs / leaks state — src/client/client.ts:677-693 (close()/waitForDisconnect()) + src/connection/connection.ts:340-367,784-823,848-877. Permanent socket handlers (which stop the queue, reset the router, clear connectInProgress, fire teardown) are wired only at the end of a successful connect(). A logout()/close()/disconnect() overlapping a still-negotiating connect (greeting/STARTTLS window) can leave connectInProgress stuck true forever (instance permanently unusable) and/or leak _logoutPromise, and disconnect()'s bare socket.destroy(undefined) fires neither handler set. Two groups independently found this from the client and connection sides. Fix: guarantee "disconnected"/teardown from every path including teardownFailedConnect() and mid-connect disconnect().

H2. postBoundaryDiscard not cleared by teardown paths → silent black-hole on reconnect — src/connection/connection.ts:340-367 (teardownFailedConnect), 784-823 (onSocketClose). These reset secure/preauthed/compression but not postBoundaryDiscard; if a socket error fires mid-boundary-negotiation, a reused instance's next connect() can have its greeting (and everything) silently dropped until greeting-timeout, with no error. Fix: reset the flag in both teardown chokepoints.

H3. Opportunistic STARTTLS may abort the whole connection on server decline — src/connection/connection.ts:720-734. The unconditional teardown-on-false after starttls() runs for both STARTTLS and STARTTLS_OPTIONAL; if starttls() ever returns false for a mere decline (vs. a genuine failure) under the optional policy, opportunistic-TLS callers lose the connection against any server not offering STARTTLS. Uncertainty: depends on starttls()'s return contract; flagged High pending confirmation (the G21 pass did not conclusively resolve it).

H4. unhandled event silently drops tagged/continuation anomalies — src/client/client.ts:1610-1614. The bridge narrows a 4-way union with instanceof UntaggedResponse || UnknownResponse, swallowing unmatched-tag TaggedResponse and unclaimed ContinueResponse — the exact protocol-desync signals the event exists to surface. No error/log fires. Fix: widen to the full union or route the two cases to the error/log channel.

H5. Empty-array SEARCH criteria bypass the empty-operand guard → wire-invalid command — src/commands/search-criteria.ts:394-433 (estimateKeyCount uses Math.max(v.length,1)), src/commands/search.ts:604-608. {keyword: []}, {header: []}, {and: []}, or nested fuzzy/or/not with an empty array compile to a bare SEARCH with zero search-keys (illegal per RFC grammar) or a malformed sub-expression — the very class the recent empty-{} fix targeted, but empty arrays slip through. Reachable via the typed public API. Fix: count empty arrays as 0; add a top-level zero-token check; regression tests.

H6. body-fld-octets/body-fld-lines reject bigint (63-bit sizes) — src/parser/structure/fetch/body.structure.ts:233-238,270-282. Only TokenTypes.number accepted; RFC822Size/BinarySize siblings correctly accept number|bigint per RFC 9051 App. D-1. A >2^32-octet body part throws ParsingError on FETCH BODYSTRUCTURE — and this contradicts the I-10 ledger claim that bigint promotion is handled uniformly. Fix: accept bigint; verify D-1's production list.

H7. Empty LIST mailbox name incorrectly rejected → breaks delimiter discovery — src/parser/structure/mailbox/listing.ts:186-190. Throws on an empty name, but LIST "" "" → * LIST (\Noselect) "/" "" (empty name) is the standard, spec-documented hierarchy-delimiter-discovery response. Not adjudicated. Fix: allow well-formed empty astring; add a regression test.

H8. Empty list block → raw TypeError that errors the whole parser stream — src/parser/utility.ts:17-88 (splitSpaceSeparatedList) + src/parser/parser.ts:79-113. A trivially malformed ( )-shaped list produces an empty block; getSpaceSeparatedStringList then dereferences undefined, throwing TypeError (not the usual typed ParsingError), which _transform delivers to done(error) — putting the Transform stream into a permanently errored state and killing parsing for the rest of the connection from one malformed line. Fix: skip/guard empty blocks and raise ParsingError.

H9. BODY[section] and BINARY[section] collide on map key → silent data loss — src/client/fetch.ts:659-717. buildFetchedMessage() keys one Map by section string only, so a single FETCH requesting both BODY[1] and BINARY[1] has the BODY result silently clobbered by BINARY. Fix: composite (section, binary) key. (Reachability depends on whether commands/fetch.ts's wire compiler — a stalled group — permits both in one request; confirm.)

H10. part.buffer() hangs forever if the stream is destroyed mid-drain — src/client/fetch.ts:35-69 (drainReadableAsync). Listens for readable/end/error but not close; destroy() emits only close, so an abandoned fetch() iterator (a break out of for await while a not-yet-awaited buffer() is draining) leaves that promise unsettled forever. Fix: add a close handler that settles (reject as known-incomplete).

MEDIUM

• M1. FETCH section-spec / binarySize strings bypass grammar validation → structural-char injection into BODY[...]/BINARY[...] — src/commands/fetch.ts:128-150,249-262 (via writer.raw()).
• M2. STORE MODIFIED UID-set expansion has no DoS ceiling (duplicates but doesn't reuse collector.ts's MAX_EXPANDED_UIDS cap) — src/commands/store.ts:213-230.
• M3. Unbounded ESEARCH range expansion is a DoS vector from server data — src/commands/search.ts:482-516. (M2/M3 + collector share one root pattern: server-supplied UID/seq ranges materialized without a bound.)
• M4. No recursion-depth limit on nested BODYSTRUCTURE/extension data → stack-overflow DoS — src/parser/structure/fetch/body.structure.ts:161-191,250-267,332-335.
• M5. Unbounded recursion parsing nested ESEARCH data — src/parser/structure/mailbox/search.ts:338-357. (M4/M5 share the "no nesting-depth cap on untrusted server data" root; a single project-wide depth guard would address both.)
• M6. MessageBodyStructure lacks the min-field-count validation Envelope has → raw TypeError on truncated data; parseParamList unguarded against undefined — src/parser/structure/fetch/body.structure.ts:208-246.
• M7. Unbalanced nested parens silently corrupt list parsing (silent data loss, negative openParenCount) — src/parser/utility.ts:90-118.
• M8. A single parse exception is fatal to the entire parser stream (amplifies H8/M6/M7) — decide + document recoverable-vs-fatal — src/parser/parser.ts:79-113.
• M9. stream()-after-buffer() before the buffer settles hands the same live stream to two consumers → silent corruption — src/client/fetch.ts (stream() at 529-541 vs buffer() at 506-521).
• M10. COMPRESS/UNAUTHENTICATE topology swap can corrupt the DEFLATE stream when a TCP segment straddles the boundary (discarded bytes may be real data, unlike the STARTTLS case) — src/connection/connection.ts:1427-1532,1574-1648.
• M11. disconnect() is a silent no-op during the implicit-TLS connect() window (checks !this.socket but not connectInProgress) — src/connection/connection.ts:848-877.
• M12. CommandQueue.hold()/release() cannot nest (plain boolean) though flushPending()'s doc claims nested support — src/connection/queue.ts:452-467. Confirm no nested-hold caller in connection.ts, or use a depth counter.
• M13. SCRAM missing describeFailure() → loses server e= diagnostics on a tagged-NO rejection (its OAUTH siblings got this exact fix) — src/sasl/scram.ts:373-431. (Diagnostic only; accept/reject decision is unaffected and fail-closed.)
• M14. Server-text CRLF/control sanitization (present in login.ts) not applied to authenticate.ts:210-227 and starttls.ts:52-74 (the latter literally builds message += "\r\n" + text) → log-injection from an untrusted/pre-auth peer.
• M15. CompressCommand omits the capability field its sibling commands declare → Layer-2 client.run(new CompressCommand()) is ungated — src/commands/compress.ts.
• M16. EnableCommand likewise has no capability field → ImapClient.run()'s capability enforcement (client.ts:1539) is silently skipped for ENABLE via the escape hatch — src/commands/enable.ts. (M15/M16 + M17 are one theme: capability/state gating gaps on the Layer-2 escape hatch.)
• M17. ImapClient.compress() performs no client-state precondition check (unlike every other verb) and conflates "no connection" with "server declined" — src/client/client.ts:2029-2046.
• M18. unauthenticate()'s post-await race handling doesn't cover a concurrent logout() (state left mislabeled) — src/client/client.ts:2183-2249.
• M19. Stale/self-contradictory comment about parseTokens' null return (references removed push() code); underlying "bare null on the unknown event" question still open — src/parser/parser.ts:116-123.

LOW (grouped; titles + refs)

Parser/robustness: partial-decode state not cleared on non-partial decode failure (parser/encoding.ts:8-130); INTERNALDATE/SAVEDATE silently produce Invalid Date (fetch/internaldate.ts, fetch/extension.ts); oversized ESEARCH COUNT/MIN/MAX silently dropped (mailbox/search.ts:308-329); unanchored ESEARCH key-format regex (mailbox/search.ts:32-34).
Connection: tautological !this.isSecure STARTTLS check (connection.ts:693-698); connected local reused for two meanings (connection.ts:592,700); onSocketError param implicitly any (connection.ts:781-783); invalid tls config silently coerced to DEFAULT (connection.ts:450-462); deflate write path ignores backpressure (compress.ts:106-109); legacy untyped deepEqual helper (utils.ts:54-169); isIsolated conflates two queueModes (queue.ts:365).
Commands: MultiAppendCommand duplicates writeAppendMessageBody (append.ts:698-722); STORE emits FLAGS () for empty array — confirm intent (store.ts:162); mechanism.step() exception discarded on abort (authenticate.ts:115-142); IdCommand silently drops non-standard field names (id.ts:145-196); GmailLabelsStoreCommand no labels validation (gmail-labels.ts:77-83); GetMetadataCommand echoes raw (non-canonicalized) mailbox, inconsistent with siblings (metadata/get-metadata.ts:143,180); SEARCH RETURN error message omits RELEVANCY (search.ts:298).
SASL: public nonce-override footgun on public factory surface (scram.ts:105-140); NFKC applied to username but not authzid (scram.ts:242,246).
Client/mailbox: enableExtensions() skips state validation on empty result (client.ts:789-808); duplicate resync-flush comment block (mailbox.ts:718-735); MailboxUpdate flags variant hand-duplicates fields (mailbox.ts:216-220); duplicated modseq/flags construction (client.ts:1828-1881); stale M1.6-era class doc comment (client.ts:99-107, reported by two groups).
Docs: garbled duplicated doc fragment (protocol/response-codes.ts:1-43); RFC9051-A-1 classification asymmetry vs. RFC4422 rows (uncertainty); CHANGELOG "121 sources" figure not independently recounted.

---

Cross-cutting themes for the maintainer

1. Untrusted-server-data hardening is the dominant risk. C1, H6, H7, H8, M2-M8 are all "a malformed/hostile server response crashes, hangs, over-allocates, or silently corrupts." A shared max-nesting-depth guard, a shared bounded UID-set expansion (reuse collector.ts's cap everywhere), and a decision on parser-stream fatal-vs-recoverable (M8) would close most of them at once.
2. Connection/client teardown races (H1, H2, H3, M10, M11, M17, M18) — the file is already heavily hardened from a prior review round, but a few teardown/topology-swap paths remain.
3. Layer-2 escape-hatch gating gaps (M14-M17) — the capability/state gate that ImapClient.run() provides is bypassed by a few commands that don't declare capability/state.

Totals

1 Critical · 11 High · 19 Medium · ~28 Low. No finding contradicts the documented adjudications; the adjudication ledger, MIGRATION.md, and README were verified accurate against code.

---

Generated by an automated multi-agent code review (orchestrator + per-group review-runner deep passes). The review-validator second-pass and 9 of 27 groups did not complete in this environment; findings are single-reviewer, self-verified by direct code tracing, and should be confirmed before acting — start with a reproduction test for C1.

---

Generated by Claude Code

[LoveAndCoding]

Addendum to the automated review — two more groups completed (lexer + test-quality)

Two review groups returned after the main report above. Both add material findings, including a new High and a confirmed production bug tests route around. The overall recommendation is unchanged (HOLD for changes), and these reinforce the two dominant themes (untrusted-server-data robustness; under-tested complex parsers).

Added HIGH

H12. Lexer swallows unrecoverable tokenization errors as "incomplete" → unbounded memory / DoS — src/lexer/lexer.ts:185-227 + src/lexer/rules/string.ts:20-25. _transform's catch unconditionally treats every tokenize() throw as "probably mid-literal, wait for more bytes." That's correct for the literal-length case but wrong for StringRule's "Unable to find end of string" (an unterminated quoted string — which per RFC can't cross a CRLF, so on an already-complete line it's terminal, not partial). The buffer is never reset and has no size cap, so a malformed/hostile server response makes the lexer accumulate all subsequent lines indefinitely (memory DoS), and a later stray " can retroactively "close" the bogus string and silently misparse large amounts of valid traffic. Fix: distinguish "needs more bytes" from "syntactically terminal" and propagate the latter via done(error).

H13. MessageHeader.mergeIn is broken and untested (confirmed production bug) — src/parser/structure/fetch/header.ts:132 (and MessageBody.mergeIn, src/parser/structure/fetch/body.ts:115). withHeader.fields.forEach(([key, val]) => ...) destructures Map.forEach's callback wrong (it receives (value, key), not [key, value]), so the real field name is discarded and the value is mis-destructured. This is the code path for a header split across multiple FETCH responses. A test (test/unit/parser/structure/fetch/body.section.test.ts:140-152) explicitly documents the bug and tests around it; no test exercises mergeIn in either direction. Fix: iterate .entries()/for...of; add direct unit tests.

Added MEDIUM

• M20. IMAPLogMessage public discriminated union isn't discriminating — "warn" is in both arms, so level === "warn" can't narrow to detail/error — src/types.ts:10-27 (re-exported public type).
• M21. Lexer.tokenize() reassigns a shrinking substring per token → potential O(n²)/whole-buffer retention on very large single-line responses (e.g. 100k-UID SEARCH) — src/lexer/lexer.ts:283-318. Benchmark on target V8.
• M22. IMAPError (legacy, Layer-1) vs ImapError (public §4) naming collision with no distinguishing comment; and Connection's connectionError type (IMAPError/ConnectionErrors) isn't exported from index.ts, so the sanctioned Layer-1 escape hatch's error channel isn't type-consumable — src/errors.ts, src/index.ts:228-230.
• M23. Inconsistent literal +-marker ({n+}) tolerance between framing layers: newline.transform.ts:47 and lexer.ts:25 tolerate it but rules/string.ts:36 doesn't → below-threshold {n+} desyncs the two layers on adversarial input — confirm reachability with a test.
• M24. No dedicated BODYSTRUCTURE unit test — only 3 incidental cases in tolerance.test.ts:312-370; test/unit/parser/structure/fetch/body.structure.test.ts is absent. This is the same parser as Critical C1 — the nested-multipart crash lives in an under-tested surface. Add systematic single/multipart/nested/malformed coverage.
• M25. Compliance-suite expectFailure: "unimplemented" annotations and "unimplemented today" comments are stale vs. driver.ts (fetch/search/uidFetch are now wired) — the aggregator flags a passing-but-stale-hint case, but the annotation bookkeeping that keeps the compliance score honest can drift. Audit test/compliance/specs/** against driver.ts.

Added LOW

• LiteralBodyStream.fed is write-only dead state (src/literal-body-stream.ts:69,121).
• Dead *-boundary branch in AtomRule — * already excluded by the atom char-class (src/lexer/rules/atom.ts:26,40-41).

Positive signal from the test-quality sample (G27)

Where tests exist, quality is high: SASL/SCRAM tests use real RFC worked-example vectors and fail-closed assertions; the wire-writer and sequence-set suites are byte-exact; STARTTLS tests use real sockets. Importantly, the compliance harness is not self-certifying — RFC text is extracted into a catalog independently of the implementation, unannotated failures default to "violation" (not "unimplemented"), and acknowledged gaps still count against the denominator. That is an honest scoring model. Caveat: only 9 of 133+ unit files and 2 of ~35 compliance specs were sampled.

Revised totals and coverage

1 Critical · 13 High · 25 Medium · ~30 Low. Groups: 19 of 27 returned complete reviews, 1 partial (ENABLE capability-gate finding, folded into M16); 7 did not complete in this environment — mailbox.ts parts 2-3, client config/facets, commands base/writer/collector, select/status/list, misc mailbox commands, and parser-misc (incl. the 15KB text.code.ts response-code parser). Those files got partial indirect coverage via adjacent groups but warrant a focused re-run before merge.

---

Automated multi-agent review addendum. Findings are single-reviewer deep-pass results (validator second-pass unavailable in this environment); confirm before acting. Start with reproduction tests for C1 (nested-multipart BODYSTRUCTURE) and H13 (mergeIn), which are the two most concrete confirmed defects.

---

Generated by Claude Code

[LoveAndCoding]

Addendum 2 — parser-misc group completed (resp-text-code / capability / THREAD)

One more previously-incomplete group returned (src/parser/structure/ misc: text.code.ts, capability.ts, untagged.ts/tagged.ts, quota.ts, thread.ts, etc.). Recommendation unchanged (HOLD for changes).

Added MEDIUM

• M26. COMPRESS=DEFLATE (and the other KIND=VALUE entries in standardCapabilityNames) is misclassified as an unknown capability — CapabilityList.add() branches on includes("=") before consulting isStandardCapability(), so those entries are dead code and COMPRESS's kind isn't in kindValueStandardCapabilityNames, yielding isUnknown = true for a widely-deployed extension. Consumers filtering/logging on isUnknown misreport it — src/parser/structure/capability.ts:365-390,186-231,49-61.
• M27. MODIFIED resp-text-code reuses the UID-only UIDSet, which rejects the legal "*" wildcard when MODIFIED rides a plain (non-UID) STORE/EXPUNGE (message-sequence context) — e.g. [MODIFIED 2:*] throws — src/parser/structure/text.code.ts:148-159, src/parser/structure/uid.ts:37-98. (Medium-confidence; verify RFC 7162 §3.8 ABNF.)
• M28. Untagged * OK/NO/BAD/BYE [code ...] lines whose resp-text-code throws lose all structure (fall through to UnknownContent) — asymmetric with the graceful "bare status word" fallback this PR added for the tagged path. A single malformed code (e.g. the UIDVALIDITY 0 server quirk, or M27) silently destroys an otherwise-informative SELECT/EXAMINE untagged OK — src/parser/structure/untagged.ts:110-151 vs src/parser/structure/tagged.ts:30-56.

Added LOW

• Several .match() methods (quota.ts, sort.ts, thread.ts, vanished.ts, urlauth.ts) slice past an unverified SP token and access tokens unguarded → raw TypeError instead of ParsingError on truncated input (currently masked by the untagged blanket try/catch).
• Unbounded recursion depth parsing nested THREAD responses (src/parser/structure/thread.ts:11-41) — same untrusted-server recursion theme as C1/M4/M5.

Coverage update

Now 20 of 27 groups returned complete reviews + 1 partial. Still incomplete (forced-async pipeline stalls): 6 groups — mailbox.ts parts 2-3 (the bulk of the 3047-line MailboxSession), client config/facets, commands base/writer/collector infra, select/status/list, and misc mailbox commands. writer.ts/collector.ts/base.ts and mailbox.ts received partial indirect coverage from adjacent groups, but a focused re-run of these six is the main outstanding gap.

Running totals: 1 Critical · 13 High · 28 Medium · ~32 Low.

---

Automated multi-agent review, final addendum. The recurring theme across the parser findings (C1, H8, H12, M4-M8, M23, M27, M28, and the THREAD/QUOTA items) is untrusted-server-data robustness — a shared nesting-depth cap, bounded expansion, and a uniform "degrade the code, keep the response" tolerance policy on the untagged path would resolve most of them together.

---

Generated by Claude Code

[LoveAndCoding]

Addendum 3 — validation pass complete; four previously-incomplete groups now returned + validated

Four of the six groups flagged incomplete in the main report finished after it was posted, each with its review-validator pass run (the step missing from the earlier comments). Recommendation unchanged: HOLD for changes, then merge-with-follow-ups. Validation earned its keep this round — it rejected one High candidate (see below), so the findings here are higher-confidence than the main report's single-pass results.

Groups now complete + validated: G4 (mailbox.ts message-op dispatch, lines 1016–2030), G7 (config/auth/capabilities/facets), G8 (command infra: base.ts/writer.ts/collector.ts — all 7 lenses, findings validated by direct re-inspection), G15 (expunge/copy/move/create).

---

Added HIGH

H14. seq.replace() skips the NOTIFY expunge-safety guard → stale MSN can replace/delete the wrong message — src/client/mailbox.ts:1610-1651 (runReplace). Every sibling seq-grain static (runStore, runGmailLabelsStore, runConvert, runCopyOrMove, runFetch) calls assertSequenceGrainSafeUnderNotify(); runReplace never does. Under an active SELECTED MessageExpunge NOTIFY registration, seq.replace(<seq>, …) can be dispatched with a sequence number that resolves to a different message by the time the server parses it — and because REPLACE atomically appends-and-removes (RFC 8508 §3.2), a stale MSN silently replaces/deletes the wrong message. This is exactly the class of hazard docs/compliance-adjudications.md's RFC5465-5.3-2 entry says the library refuses everywhere — but REPLACE was missed, and the omission is not itself adjudicated. Validator: Valid. Fix: add the hasActiveNotifySelectedMessageExpunge() refusal for kind === "seq", mirroring the siblings; add coverage.

Added MEDIUM

• M29. UID EXPUNGE capability gate ignores IMAP4rev2's base-protocol fold-in — src/client/mailbox.ts:1809-1817 gates on !hasCapability("UIDPLUS") alone; ExpungeCommand.capability is a bare "UIDPLUS" (src/commands/expunge.ts:85-87). Sibling rev2-absorbed verbs OR-gate correctly (idle() :1102, unselect() :1327, MOVE :1866-1867). The repo's own catalog (test/compliance/catalog/ext/rfc4315.ts:62) confirms RFC 9051 §6.4.9 absorbs UID EXPUNGE into rev2-base. A pure-IMAP4rev2 server not separately advertising UIDPLUS gets every expunge(uids)/seq.expunge() rejected with CapabilityError. Validator: Valid (fails closed, so Medium not High). Fix: OR in IMAP4rev2; widen ExpungeCommand.capability to the array form.
• M30. [CAPABILITY …] resp-code reported as "CAPABILITIES", not the wire keyword — src/commands/collector.ts:111,291-295. name is derived from CapabilityTextCode.kind (a plural display label, explicitly documented in text.code.ts as distinct from the singular wire keyword), so any consumer of ResponseCollector.codes() / ServerNoError.code matching code.name === "CAPABILITY" silently fails. Found by two independent lenses; confirmed by direct re-inspection. Fix: emit the literal "CAPABILITY" in that branch.
• M31. ResponseCollector.settle() lacks the terminal-state guard abort() has — src/commands/collector.ts:406-410 vs 436-443. settle() has no if (this.settledFlag) return;, though its own doc comment claims the "same single-terminal-state discipline." A settle() landing after an abort() (teardown racing the final tagged response) overwrites taggedResp while leaving abortErr set, so tagged() and live() disagree about whether the command succeeded. Confirmed by direct re-inspection. Fix: add the first-wins guard so the invariant is self-enforced, not caller-dependent.
• M32. expandUidSet() throws a bare RangeError, violating the "every public promise rejects with ImapError" invariant — src/commands/collector.ts:55-79, reached unguarded from CopyCommand.accept() (copy.ts:90), MoveCommand.accept() (move.ts:111,123), and defaultOnError (base.ts:211). A hostile/non-conformant server sending a >1M-wide COPYUID/MODIFIED range surfaces a RangeError to the caller instead of an ImapError subtype (worst case, an uncaught synchronous throw on the live untagged path). Validator: Valid (Low-Medium; the actionable fix lives in shared infra — wrap toTypedResponseCode()).
• M33. Two resp-code citations attribute arguments to RFCs that don't define them — src/commands/collector.ts:191-211. APPENDLIMIT is treated as an RFC 7889 resp-code, but 7889 defers over-limit APPEND to TOOBIG (RFC 4469) and defines no such resp-code; BADCOMPARATOR parses a trailing charset arg that RFC 5255 §4.9's ABNF (resp-text-code =/ "BADCOMPARATOR", argument-less) doesn't have. Both corroborated by the repo's own compliance catalog. Fix: drop/rename the speculative branches or correct the citations.
• M34. ExpungeCommand is omitted from the src/commands/index.ts barrel export — all 9 sibling commands are exported; this one is reachable externally only via the documented client.run(new ExpungeCommand(...)) escape hatch (README:187-190), which the omission silently breaks. Validator: Valid.
• M35. No authzid config field → EXTERNAL/ANONYMOUS SASL can't carry non-empty trace info — src/client/config.ts:30-41,186-190. validateAuth() also throws unless pass/accessToken is set, with no carve-out for mechanisms that need neither, so the config-embedded path rejects them outright. A 1.0 that fully implements both mechanisms exposes no first-class way to use their defining feature. Validator: Valid. Fix: add authzid?: string, thread it into the SaslContext, relax the pass/token requirement for no-secret mechanisms.
• M36. idle()'s IdleController isn't torn down on session close — src/client/mailbox.ts:1100-1119. Unlike updates() (which subscribes to "closed"), idle() never reacts to close/reselect; after a reselect mid-idle it can silently idle on the newly-selected mailbox. Validator: Valid (Medium — reachable without caller misuse).

Added LOW (grouped)

fetchOne()'s doc comment contradicts fetchOneOf()'s actual (protect-not-destroy) behavior (mailbox.ts:1039-1049); astring()'s "never throws on content grounds" is false for NUL (writer.ts:553-559); writer atom-validation over-excludes "[" (legal ATOM-CHAR) and mis-cites it to RFC atom-specials (writer.ts:82-104); SEQUENCE_SET_RE is a character-class check the docs oversell as grammar validation (writer.ts:305,728-737); stale "future milestone / not-yet-wired" comments in base.ts (states/capability are enforced) and "(stubbed)" mailbox-codec comment; CreateCommand's empty-USE () refusal misquotes RFC 6154 §6 (the ABNF list is optional) — also duplicated in the test title test/unit/client/mailbox-verbs.test.ts:83; config.ts allowInsecureAuth coerces instead of validating; defaultCandidates() silently omits CRAM-MD5; plus assorted naming/dup/declare-comment consistency items across the four groups.

---

Validation rejected one candidate (working as intended)

REJECTED — deferred-dispatch (chainFamily) "wrong-mailbox" race (originally drafted High). The validator traced it and found it timing-impossible: SelectCommand.queueMode is "serial", so a reselect enqueued behind an in-flight same-family command cannot even write bytes until that command drains; and performSelectOrExamine() holds state at "authenticated" for the whole SELECT round-trip, so the deferred continuation (firing microtasks after the prior command settles) hits ImapClient.run()'s state gate and throws StateError long before SELECT completes. No data-integrity hazard; at most a harmless defensive _closed re-check. Not included as a defect.

---

Coverage after this pass

24 of 27 groups complete (23 with a validator pass; G8 validated by direct re-inspection). Still without a dedicated findings pass: 2 groups — select/status/list and the mailbox.ts tail (lines 2031–3047, the SeqFacet delegator + updates()/fetch drivers). Both received full context-gathering, and the mailbox tail's fetch-driver surface was covered indirectly by the fetch/idle/state group (H9/H10/M9). Candidly: further sub-agent fan-out was halted by the account's monthly spend limit (one runner terminated on it mid-pass) — those two groups are the remaining gap, best closed by a focused re-run once budget allows.

Final running totals

1 Critical · 14 High · ~36 Medium · ~45 Low, across 24 reviewed groups. No finding contradicts the documented adjudications; the adjudication ledger, MIGRATION.md, and README were verified accurate against code. Start remediation with C1 (nested-multipart BODYSTRUCTURE), H13 (mergeIn), and H14 (seq.replace NOTIFY guard) — the three most concrete confirmed defects.

---

Automated multi-agent review — validation pass. Findings validated by review-validator sub-agents (or, for G8, direct re-inspection); one High candidate was rejected on validation. Two groups remain uncovered due to a spend-limit halt, disclosed above rather than papered over.

---

Generated by Claude Code

[LoveAndCoding]

Addendum 4 — coverage complete: final two groups reviewed + validated (27/27)

The two groups still open after Addendum 3 — select/status/list and the mailbox.ts tail (2031–3047) — are now reviewed. Because the account spend limit blocked further agent fan-out, I ran these as direct single-reviewer deep passes with inline validation (each candidate checked against the repo's own compliance catalog before standing — the same catalog the suite extracts RFC text into). That validation step rejected one candidate and confirmed one, below. All 27 groups are now covered. Recommendation unchanged: HOLD for changes, then merge-with-follow-ups.

Added MEDIUM

M37. RETURN (STATUS …) gate ignores IMAP4rev2's LIST-STATUS fold-in — src/commands/list.ts:452. The outer gate is statusItems.length > 0 && !caps.has("LIST-STATUS"), with no || caps.has("IMAP4rev2"). The repo's own catalog is explicit: test/compliance/catalog/ext/rfc5819.ts:41 — "REV2-CORE ADJUDICATION … RFC 9051 folded LIST-STATUS into rev2 core." So a bare-IMAP4rev2 server that doesn't separately advertise LIST-STATUS has every list({ returnStatus: […] }) call rejected with CapabilityError. This is internally inconsistent with the codebase's own established handling of the same rev2-fold-in pattern: the per-item STATUS gate already ORs in rev2 (status.ts:80, SIZE → STATUS=SIZE || IMAP4rev2), NamespaceCommand uses capability = ["NAMESPACE","IMAP4rev2"] (namespace.ts:44), and MOVE/UNSELECT/IDLE all OR-gate — only the LIST-STATUS outer gate was missed. Same class as M29 (UID EXPUNGE). Fails closed, so Medium. Fix: !caps.has("LIST-STATUS") && !caps.has("IMAP4rev2").

Added LOW

L. Attribute-algebra step order lets a contradictory listing keep both child-state attributes — src/commands/list.ts:140-151. The \HasChildren+\HasNoChildren contradiction-collapse (lines 141-144) runs before the \NoInferiors ⇒ \HasNoChildren inference (145-147). A server sending the (self-contradictory) pair \NoInferiors \HasChildren therefore ends with both \HasChildren and \HasNoChildren in the normalized set, since the collapse already ran before \HasNoChildren was added. Only triggers on server-contradictory input and the result is arguably acceptable ("report what the server said"), but the algebra's stated intent is to prevent exactly that pair. Fix: re-run (or reorder) the contradiction check after the implication step.

Validation rejected one candidate (working as intended)

REJECTED — "SPECIAL-USE gate missing IMAP4rev2 fold-in." I suspected list.ts:470's !caps.has("SPECIAL-USE") (no rev2 fallback) was the same bug as M37. The catalog disproves it: test/compliance/catalog/ext/rfc6154.ts:41 — "REV2-CORE ADJUDICATION … RFC 9051 (IMAP4rev2) did NOT absorb RFC 6154's special-use surface into core … RFC 6154 remains a standalone extension under rev2." Gating SPECIAL-USE on its own capability with no rev2 fallback is therefore correct. Not a finding.

Verified clean (no findings)

• mailbox.ts tail (2031–3047): the refcounted live-update driver's construction-race guards (ST2/ST3 — placeholder-write + released flag) correctly prevent orphaning the IdleController under every interleaving traced; createUpdatesIterator's cleanup paths are symmetric (the 147ce7e fix closed the one real listener leak; return()/throw()/onClosed all unsubscribe idempotently and queued updates still drain post-close); driveFetch/fetchOneOf/drainAbandoned correctly use raw .next()+protectLiveParts() (never .return()) so a returned message's live parts survive while the command still settles. SeqFacet is a pure delegator.
• select.ts: CONDSTORE gated on plain advertisement, QRESYNC on the _enabled-aware probe, the combined-refusal is the adjudicated SF3 decision, and the qresync-param grammar guards (uidvalidity nz-number, seqMatch-requires-knownUids, pre-write validation) are all correct.
• status.ts: the per-item gate table correctly encodes every rev2 fold-in (SIZE, DELETED) and correctly excludes MAILBOXID (OBJECTID is not rev2-core, per catalog); name attribution is by-mailbox, never most-recent-wins.
• lsub.ts / namespace.ts: LSUB correctly ungated (rev1 base grammar), RLSUB/RLIST/NAMESPACE gates correct.

Final coverage & totals

27 of 27 groups reviewed. 23 groups carried a review-validator sub-agent pass; the remaining 4 (G8 command-infra + these last two) were validated by direct re-inspection against source and the compliance catalog after the spend limit halted agent fan-out. Across the full review, validation rejected 3 candidate findings (the chainFamily race, the SPECIAL-USE gate, and — in earlier group passes — assorted mis-scoped items) and corrected numerous priorities, which is the point of running it.

Final totals: 1 Critical · 14 High · ~37 Medium · ~47 Low. No finding contradicts the documented adjudications; the adjudication ledger, MIGRATION.md, and README were verified accurate against code. Remediation priority remains C1 (nested-multipart BODYSTRUCTURE), H13 (mergeIn), H14 (seq.replace NOTIFY guard).

---

Automated multi-agent review — final coverage-closing pass. The last two groups were reviewed inline (single-reviewer + catalog-validated) because the account spend limit blocked further sub-agent runs; that constraint is stated rather than hidden.

---

Generated by Claude Code

[LoveAndCoding]

Second review addressed — the Critical, all 14 High, and the Medium/Low tier

Thank you for the second, much wider pass (and for the candid coverage/validation disclosures — they made triage faster). Everything actionable is fixed. Same discipline as before: every finding verified against the code before any change, each real fix landed with a unit test and an undo→red→redo→green revert-verification, and the compliance matrix stayed byte-identical throughout (1138 pass / 6 adjudicated violations / 2 adjudicated-unimplemented / 451 untestable / problems: []). Unit tests 2053 → 2258; typecheck, eslint (0 errors), and compliance green at every merge.

Seven commits, file-disjoint clusters:

• 06c414c parser fetch / BODYSTRUCTURE · a788113 lexer / capability / types · 46bdb43 parser-core resilience · 846e3dd search / SASL / config · 229038b message-op lifecycle · f6d15af connection / client teardown · dde8139 barrel + RFC 6154 cleanup

Critical

• C1 nested-multipart BODYSTRUCTURE crash — fixed. And while fixing it I found the MESSAGE/RFC822 detection the finding cited as the "correct pattern" was itself broken (its [1].length===1 check is true for both single- and multi-part shapes, silently misparsing every embedded single-part body). One shared parseBodyStructureFromParts() with the real discriminant (is the first field a parenthesized list) now serves all three call sites. New 18-test suite.

High — all 14 fixed

H1 connect()-teardown race (teardownFailedConnect now emits disconnected; mid-connect disconnect() drives the same abort path) · H3 opportunistic-STARTTLS decline no longer tears down the connection · H4 unhandled event passes the full response union · H5 empty-array SEARCH criteria now throw · H6 body-fld-octets/lines accept bigint · H7 empty LIST name (LIST "" "" delimiter discovery) accepted · H8/M8 a malformed line degrades to an unknown event instead of permanently erroring the parser stream (the linchpin — makes the whole "malformed line" family non-catastrophic) · H9 BODY[section]/BINARY[section] composite key · H10 buffer() rejects on destroy instead of hanging · H12 lexer no longer buffers forever on an unterminated quoted string (terminal UnterminatedStringError + a 10 MiB backstop) · H13 mergeIn iterates .entries() correctly (the confirmed production bug the test routed around — that test now asserts correct behavior) · H14 seq.replace() applies the NOTIFY expunge-safety guard like every sibling.

Medium — addressed

M1 section-spec injection validated · M2/M3 STORE-MODIFIED & ESEARCH range expansion now share the bounded-expansion cap · M4/M5 recursion-depth caps (BODYSTRUCTURE, ESEARCH, THREAD) · M6 truncated-BODYSTRUCTURE typed guard · M7 unbalanced parens → ParsingError · M9 stream()-after-buffer() single-consumer · M10 COMPRESS boundary now preserves straddling bytes into the fresh inflate (rather than discarding its own opening bytes) · M11 disconnect() works during the implicit-TLS window · M13 SCRAM describeFailure() · M14 server-text sanitization in authenticate/starttls · M15 CompressCommand capability field · M17 compress() state precondition · M18 unauthenticate()/logout() race → typed StateError · M20 IMAPLogMessage union now discriminates · M23 {n+} layer consistency · M26 COMPRESS=DEFLATE no longer misclassified unknown · M27 [MODIFIED 2:*] accepts the wildcard · M28 untagged resp-code tolerance (mirrors the tagged path) · M29 UID EXPUNGE OR-gates IMAP4rev2 · M30 [CAPABILITY] reports the wire keyword · M31 collector settle() terminal guard · M32 expandUidSet → ImapError on the resp-code path · M33 dropped the fabricated APPENDLIMIT branch / corrected BADCOMPARATOR comment · M34 all missing commands added to the ./commands barrel · M35 authzid config support (end-to-end) · M36 idle() tears down on session close/reselect · M37 RETURN (STATUS) OR-gates IMAP4rev2.

Low — addressed

NFKC on authzid, allowInsecureAuth validation, encoding partial-decode leak, dead deepEqual/AtomRule * removed, INTERNALDATE/SAVEDATE reject Invalid Date, address-tuple guard, list attribute-algebra order, the RFC 6154 §6 empty-(USE ()) message corrected (your catalog's own ABNF brackets the list optional — reframed as an honest client-side guardrail), stale doc comments swept, and more.

Verified not a bug / disproven (matching your validator's spirit)

• H2 postBoundaryDiscard — already reset by both teardown chokepoints.
• M21 tokenize O(n²) — benchmarked linear (V8 SlicedString makes the tail-slice O(1)); added a perf regression guard instead of a rewrite.
• M16 ENABLE capability gate — an OR-with-rev2 gate breaks this repo's own fixtures (servers advertising bare IMAP4rev1 that genuinely support ENABLE, as real servers do); left as-is with the trade-off documented in-file.
• Four parser .match() truncation Lows (quota-adjacent) — already guard truncated input.
• Empty FLAGS () (legal flag-list grammar); CRAM-MD5 omission from auto-select (spec §9.3, still usable via explicit mechanisms).

Deferred, explicitly

• Public SCRAM nonce-override footgun (M-tier Low): the compliance harness imports it via the public ./sasl path for deterministic RFC vectors, so a rename/reroute is a self-contained follow-up rather than a bundled change.
• Still open from round one: the escape-hatch cleartext-credential plumb-through and instant graceful-FIN detection.

M10's fix required touching newline.transform.ts (the byte-buffering state is private there); no other cluster touched that file. Ready for another look whenever you are.

---

Generated by Claude Code