Skip to content

feat(connector-imap, service-mailbox): the IMAP mail source — a read-only IMAP connector and the mailbox sync that feeds the ingest seam - #23303

Merged
objectstack-fleet[bot] merged 14 commits into
mainfrom
claude/issue-22871-s2-connector-imap
Oct 14, 2026
Merged

objectstack-fleet[bot] merged 14 commits into
mainfrom
claude/issue-22871-s2-connector-imap

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #22871
Clause-②: yes (a new published package, @objectstack/connector-imap, and its exports; the mailAssociation liveness row goes live)

Stage 2 of #22871: connector-imap feeds the stage-1 ingest seam

Stage 1 (PR #23250, 82581819b) shipped the ingest seam: MailIngestService.ingest turns one raw RFC 822 message into one sys_mail_message row, with dedupe, threading, association, the pool and suppression. Nothing fed it. This PR adds the mail source: a new package, @objectstack/connector-imap, and an IMAP sync in @objectstack/service-mailbox that polls each mailbox's folder above a stored UID cursor and hands every message to the seam. Stage 1's dedupe, threading, matching, re-match and suppression are reused unchanged (ruling 6116489638: Q1 A, Q3 C phase 1, Q4 A and Q5 A are not touched).

Attachment extraction, the card's last scope item, moves to #23307 (Blocked-by: #23306, the server-side sys_attachment writer), under the seat's ruling B (6151023438).

@objectstack/connector-imap (new): protocol and auth only

  • openImapSession({ account, credentials, timeoutMs?, maxLiteralBytes? }) opens a read-only IMAP4rev1 session:
    • examine(folder) answers uidValidity, uidNext and exists. It uses EXAMINE and never SELECT. A folder opened without a UIDVALIDITY is refused.
    • listMessages(fromUid) and fetchMessages(uids) fetch with BODY.PEEK[], so no message is ever marked \Seen.
    • logout() and close() end the session.
  • The account and credential abstraction is shaped for the per-user mailbox (decision 4).
    • MailAccount is { id, username, imap: { host, port?, security? } }, one identity per account, never a process-wide setting.
    • MailCredentialProvider.credentialFor(account) is resolved each time a session opens and never cached. A per-user provider can therefore read a user's stored credential and refresh an OAuth token without changing the connector.
    • The endpoint sits under imap, so the same account can later name a calendar endpoint. The design record shares one account between the mail and calendar engines.
    • staticCredentials(credential) is the shared mailbox's implementor: a fixed credential the host hands over.
  • Auth:
    • a password uses AUTHENTICATE PLAIN (with SASL-IR when offered), otherwise LOGIN, with a synchronizing literal when quoting cannot carry the password;
    • an oauth2 token uses AUTHENTICATE XOAUTH2, including its error-continuation flow;
    • a server that announces LOGINDISABLED and offers no PLAIN is refused before anything is sent.
  • Transport: implicit TLS by default, with the certificate verified. security: 'none' is refused to any host but loopback, before a socket opens.
  • Shape: it mirrors the existing connectors' package shape: manifest, LICENSE, tsconfig.json, vitest.config.ts and co-located tests. It has no runtime dependencies. It registers no connector_action handler and ships no plugin, because nothing in a flow calls IMAP; the mailbox service does.
  • @objectstack/connector-imap/testing exports ImapServerDouble, the in-process server the tests run against. It is a separate entry with its own tsup.config.ts.

@objectstack/service-mailbox: the sync half

  • Composition: new MailboxServicePlugin({ imap: { accountFor, pollIntervalMs?, batchSize?, maxMessagesPerRun?, maxMessageBytes? } }) schedules the interval job mailbox:imap-sync on the composed job service at kernel:ready. destroy() cancels the job and closes any open session.
    • The capability boot constructs the plugin with no options, and then no mailbox is synced. The boot says so once, at info.
    • With imap but no job service, the boot warns. plugin.syncService.syncAll({ isSystem: true }) is the manual path.
    • A run in which a mailbox could not be synced resolves { outcome: 'degraded' } for the job service (the JobRunOutcome contract).
  • MailboxSyncService, per mailbox:
    • it asks accountFor(mailbox) for the account and credential (null means not synced);
    • it opens the session, reads the folder's UIDVALIDITY, and lists the UIDs above the cursor;
    • it fetches them batchSize at a time and hands each to ingest under the system context, in UID order.
  • The cursor lives on sys_mailbox in four new readonly fields, written by the sync only: imap_folder, imap_uid_validity, imap_last_uid and imap_synced_at. That is a widening of a published object, declared in the changeset.
    • A cursor taken under another UIDVALIDITY, or in another folder, is discarded and the folder is read from its start. Messages already stored answer duplicate, so nothing doubles.
    • The cursor moves after each batch, past every message stored, already stored, suppressed or skipped. A store failure stops the run below the failed message, so the next run retries it.
    • A message above maxMessageBytes (32 MiB by default), or one the seam refuses as unreadable, is skipped so it cannot stall the mailbox. It is logged at error once, naming its UID, with the consequence and the fix.
    • One mailbox is never synced twice at once in one process. Two processes waste a fetch and store nothing twice, because the dedupe key is unique.
  • No credential is stored on any object. The account and credential come from the host's accountFor, the way every open connector takes its credential (measured below). system-context.mdx gains row 47c for the sync's two system-only entry points.

Stage 1's four carried items (6145459294)

  1. mailAssociation → live (packages/spec/liveness/field.json). authorWarn and authorHint are dropped. The authorWarnedProperties('field') pin becomes ['conditionalRequired'], and the field state counts go from 94 live / 4 planned to 95 / 3. The evidence anchor is match-records.ts#MailRecordMatcher rather than the dispatch's #MailRecordMatcher.match, because check:liveness refuses a dotted anchor ("an anchor must be one identifier"). The prose names match, through flaggedFieldsOf. This flip is its own first commit (610e112bd). Under the seat answer 6151023438 it lands first on its own PR, fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314. This PR keeps the commit, which becomes a no-op once fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314 is on main and main is merged here.
  2. Attachment storage: not built here. It moves to service-mailbox: store an ingested message's attachments, and each forwarded message part as an .eml attachment, through the server-side writer: the attachment item of #22871, chained #23307 under ruling B (6151023438). See Measurements, H5.
  3. message/global (RFC 6532): it is now read as an attachment source, like message/rfc822, including a base64-encoded part. It is pinned in parse-message.test.ts.
  4. MAX_FORWARDED_PARTS = 20 is named in the service README, with what happens past it.

Measurements (Zone 2), at the merge base 175e8d414

  • H1, the IMAP client: an own minimal implementation, zero runtime dependencies.
    • imapflow@2.4.0 (MIT, ESM and CJS, 1.75 MB unpacked) pins 8 exact-version dependencies: pino with its worker-thread transport, socks, iconv-lite, libmime, libqp, libbase64, encoding-japanese and @zone-eu/mailsplit. That is a lot for the five commands a read-only mirror sends.
    • imap@0.8.19 (node-imap) is unmaintained.
    • The subset this needs is CAPABILITY, LOGIN / AUTHENTICATE, EXAMINE, UID FETCH and LOGOUT, plus literal framing. That is about 1,000 lines with the wire reader and the account types, and MIME stays with stage 1's postal-mime.
  • H1, the server double: an own in-process double, plus a one-off interop probe against an independent strict server.
    • hoodiecrow-imap was dormant from 2.1.0 (2016) until 3.x on 2026-10-06/07, and was deprecated hours later as "Renamed to imapkit".
    • imapkit was created 2026-10-07 and had 11 releases in two days; 5.0.1 is five days old.
    • wildduck needs MongoDB and Redis.
    • So no third-party package is added, even as a dev dependency. ImapServerDouble shares no parser or constant with the client.
    • The client was probed once, outside the tree, against both imapkit@5.0.1 and hoodiecrow-imap@3.3.1. Both are strict RFC 3501 servers by the postal-mime maintainers. 18 checks, all PASS on both:
      • LOGIN, AUTHENTICATE PLAIN with and without SASL-IR, a UTF-8 password, and a password carrying a double quote and a backslash;
      • XOAUTH2 success and its error-continuation refusal, a wrong password as authentication, and LOGINDISABLED refused before sending;
      • EXAMINE with UIDVALIDITY and UIDNEXT, UID FETCH sizes, the last-UID edge case of a range ending in the star, byte-exact 7-bit and 8-bit bodies, and INTERNALDATE with an offset;
      • a modified UTF-7 folder name, LOGOUT, and no \Seen left after a fetch. That last check is read through imapkit's control API; hoodiecrow has none, so it is NOT MEASURED there.
  • H2, credentials:
    • All four existing connectors take a static credential from the host composition: connector-slack token, connector-rest authentication, connector-mcp env/headers, and the showcase's process.env.SLACK_BOT_TOKEN. "OAuth2 install/refresh, credential vaulting, and multi-tenant connection lifecycle are the enterprise tier" is the slack connector's own header.
    • The sync reuses that path (accountFor plus a credential provider). No secret store is added, and no secret field either. The engine's encrypted secret channel (resolveSecretField) is the natural store for the per-user phase.
  • H3, cursor state: sys_mailbox held no cursor. Its own header said the account, credential and cursor "are added by the ingest half". Four fields are added (the widening above). The account and credential are not stored, by H2.
  • H4, scheduling: this reuses IJobService.schedule(name, { type: 'interval' }, handler) resolved at kernel:ready, plus cancel in destroy(). That is plugin-approvals' SLA clock pattern. No timer of its own; check:plugin-teardown-shape is green.
  • H5, attachment storage: no server-side writer exists.

Hardening round (seat note 6151344448), commit 4488d1171

  1. Bounded accumulation. The session refuses, as the new ImapError kind oversized, and closes the connection when:
    • one response carries more than IMAP_MAX_RESPONSE_LITERALS (32) literals;
    • the untagged responses to one command total more than maxCommandBytes (default 128 MiB; each response is charged 256 bytes beyond its own size).
      The existing line and literal refusals moved from protocol to oversized. The package is unreleased, and the changeset lists the kinds.
  2. maxLiteralBytes follows maxMessageBytes. The sync opens the session with maxLiteralBytes = maxMessageBytes + SYNC_LITERAL_ENVELOPE_BYTES (64 KiB), and maxCommandBytes = max(128 MiB, batch budget + maxMessageBytes + envelope).
    • An oversized refusal during a batch fetch reopens the session (checking UIDVALIDITY), re-reads the batch one UID at a time, and skips the offender as too_large.
    • A body that arrives within the envelope but above the bound is skipped too.
    • The cursor moves past a skipped message, and the next message lands.
  3. A batch byte budget. Batches are bounded by maxBatchBytes (default 64 MiB, never below one message's bound) as well as by count. A message the listing says is oversized costs 0, because it is never fetched; an unsized one costs the whole bound.
  4. The \Seen sentence in testing.ts's header and in the connector README now says what the code does: only BODY[] under SELECT marks a message seen, and EXAMINE never changes state.
  5. The changeset drops '@objectstack/spec': patch and the spec bullet, because fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314 carries them. It also drops the liveness clause from its own Clause-②: yes line. The llms.txt package count (70 to 71) rides the fixed-group release.

main was merged in via os-regen-merge.sh (c33c6dc67), because GitHub reported a census conflict. main's side was taken for the two census files, which were then regenerated.

Fix round after the delta review FAIL (6154709411), commits a11f7c57e, 2725aacca, 5e075ec2f

  1. The cursor passes every skipped message. skip() now advances result.lastUid itself, so all three too_large paths pass the cursor, including a message that is the last of its batch and run. Four offender-last pins cover: listed above the bound; refused alone with the size understated; refused alone with the size omitted; above the bound once read. The second run fetches nothing and logs nothing new.
  2. A literal is charged at its announcement. At each {n} announcement, before any octet is read, the reader charges the running response against maxCommandBytes. One hostile response now holds at most maxCommandBytes plus one line. A pin uses the double's new withheld-literal mode.
  3. The changeset's openImapSession line gains maxCommandBytes?.
  4. The service README names the listing ceiling. At the default bound, a folder with more than about 450,000 messages above the cursor is refused on the listing. A host raises the bound through maxBatchBytes; paging is the per-user phase's.

Carried, not fixed: a run's time is unbounded under a server that trickles a literal. Memory stays bounded. Carrier: the per-user mailbox phase of #22871.

main was merged in again (671693ad6) after PR #23314 landed as 777eed2bc. The one conflict, the mailAssociation row's note, resolved to main's side. This PR's liveness and lint-test files now equal main's, so 610e112bd is a no-op.

Size and landing

47 files, +4105 / -63 = 4168 changed lines against the merge base (LICENSE is 202 of them). That is over the 3,000-line threshold, so under the seat answer 6151023438 this PR takes the Tier H route: the seat's ACCEPT and an at-tier contract review, then it waits for a human merge. The seat does not arm auto-merge. The diff touches no governed surface.

Verification at 671693ad6 (merged with origin/main at 430e5f5ad)

The figures at 671693ad6 come first, then those at 44aabc877, then the reading at 27aa15166, on which the first contract review PASSed (6151327338).

  • At 671693ad6 (under os-verify-lock):
    • a full build, 75 of 75;
    • connector-imap 42 passed; service-mailbox 101 passed; both typechecks exit 0;
    • spec check:liveness exit 0, and the lint liveness pin 100 passed;
    • ablations F1 (no cursor advance in skip(): 4 red) and F2 (no per-announcement charge: 1 red), each restored to blob == HEAD;
    • the interop run on the new dist: imapkit@5.0.1 and hoodiecrow-imap@3.3.1, 18 of 18 each;
    • dispatch-gates --ran: 130 derived, 130 run, all exit 0.
  • At 44aabc877 (under os-verify-lock):
    • a full turbo build, 75 of 75 (the first post-merge build was OOM-killed under contention; only the green re-run is cited);
    • spec check:generated, all 14 current;
    • @objectstack/connector-imap: 2 files, 41 passed; @objectstack/service-mailbox: 8 files, 97 passed; both typechecks exit 0;
    • six more ablations (N1 to N6: the literals bound, the command bound, maxLiteralBytes not passed, no one-at-a-time re-read, no byte budget, no post-fetch length check), each red as predicted and restored to blob == HEAD;
    • the interop run on the hardened dist: imapkit@5.0.1 and hoodiecrow-imap@3.3.1, 18 of 18 each;
    • dispatch-gates --ran: 132 derived, 132 run; 131 exit 0, and check:platform-checklist exits 1 with main's 8 findings;
    • ESLint over the hardening round's 12 changed lintable files: 0 errors, 0 warnings;
    • spec test:repo was not re-run, because the hardening touches no spec file.
  • At 27aa15166:
  • Suites:
    • @objectstack/connector-imap: 2 files, 37 tests passed.
    • @objectstack/service-mailbox: 8 files, 91 tests passed (stage 1 had 64). These run on a real ObjectQL over driver-sql and better-sqlite3 :memory:, plus a booted ObjectKernel.
    • @objectstack/lint lint-liveness-properties.test.ts: 100 passed.
    • pnpm --filter @objectstack/spec test:repo: 58 files, 1007 passed.
  • Typecheck: both packages exit 0. tsc --listFiles counts 2 of 2 and 8 of 8 test files.
  • Ablations: 11, each through scripts/ablation-replace.mjs. Each anchor hit once, each blob change was proven on disk, each file was restored to blob == HEAD with git diff HEAD empty, and the tests ran on mutated source (vitest aliases every workspace import to src). All 11 went red as predicted:
    • clause 1: the sync hands nothing to ingest — 14 tests red;
    • clause 2: the cursor is never applied (5), the dedupe key differs per fetch (2), and a changed UIDVALIDITY keeps the old cursor (1);
    • clause 3: re-match adds no link — 1;
    • clause 4: suppression removed — 1;
    • read-only: BODY[] instead of BODY.PEEK[] (1), and SELECT instead of EXAMINE (1);
    • plaintext to a non-loopback host accepted — 1;
    • message/global not read — 2;
    • the cursor never saved — 5.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derives 132 families on this head. All 132 were run, one log each, and --ran reconciles: "132 derived, 132 run, 0 NOT-MEASURED, 0 UNRUN".
    • 131 exit 0, among them check:dual-build-cjs-loads, check:published-files, check:i18n, check:system-context-census, check:tenant-audit-census, check:lockstep-package-count, check:changeset-no-major and check:generated after a real spec build.
    • pnpm check:platform-checklist exits 1 with 8 findings, the same 8 as on the merge base (Acceptance notes).
    • Extra, not derived: pnpm check:i18n-coverage exits 0, with none new.
    • The tree is 3 commits behind origin/main 9c7cfbf0a. Those commits touch none of this diff's files. One of them changes scripts/engine-double-contract.pinned.json, which the derivation reads; its gate ran green on this tree's copy.
  • Lint (narrowed): eslint --no-inline-config --format json over the 33 changed lintable files gives 33 files, 0 errors, 0 warnings, 0 fatal.
    • The population is eslint.config.mjs files **/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}; the rest of the diff is md, mdx and json. The count is read from the JSON.
    • Invariance: the config enables no type-aware linting (no parserOptions.project), so no untouched file's verdict can move.
  • turbo ls --affected is not used for any narrowing claim here.

Acceptance notes

  • check:platform-checklist is red on the merge base itself (175e8d414, run in a throwaway worktree): the same 8 ABSENT SYMBOL / SYMBOL ANCHORS LOST findings in docs/qa/platform-checklist/areas/{access-security,attachments-storage,search}.json. This diff touches none of its inputs. Not fixed here.
  • The stage-1 changeset (.changeset/22871-mail-ingest-seam.md) still says no mail source feeds the seam. It is not edited; this PR's changeset says it supersedes that line.
  • The sys_mailbox row itself (organization, address, owner) still has no administration door. Server code writes it under the system context, as after stage 1. The README says so.
  • pnpm-lock.yaml: pnpm install added the two workspace importer entries and also flipped one unrelated vitest peer variant (esbuild 0.28.2 ↔ 0.28.1), the normalization stage 1 met in the other direction. The lockfile delta was rebuilt from the base lockfile with only the two importer entries (15 lines), and pnpm install --frozen-lockfile validates it.

维护者速读(草稿)

改了什么

  • 新增发布包 @objectstack/connector-imap。它只做协议和认证:以只读方式连接一个 IMAP 邮箱,读出邮件原文。用 EXAMINE 打开文件夹,用 BODY.PEEK 读取,所以不会把邮件标成已读,也不改动邮箱。
    • 支持密码(PLAIN / LOGIN)和 OAuth 令牌(XOAUTH2)。
    • 默认 TLS 并校验证书。明文连接只允许连本机。
    • 对外部服务器设了内存上限:单行、单个邮件体、单个响应、单条命令的累计响应,超过就拒绝并断开。一个恶意或出错的服务器拖不垮本进程。
    • 账号与凭据按"每个用户一个邮箱"的形状设计,共享邮箱是第一个使用者。以后接个人邮箱是扩展,不是重写。
    • 无任何运行时依赖。
  • @objectstack/service-mailbox 新增邮箱同步。
    • 部署方组合插件时提供"哪个邮箱用哪个 IMAP 账号和凭据"(imap.accountFor),插件把同步注册成定时任务,定期把新邮件交给阶段 1 已上线的接收入口。
    • 去重、线程、按邮箱字段自动关联到客户/线索记录、未匹配进池、自动回复与退信过滤,全部沿用阶段 1,没有改动。
  • sys_mailbox 新增 4 个只读字段,记录同步进度:文件夹、UIDVALIDITY、最后同步的 UID、最后同步时间。凭据不存数据库。
  • 顺带完成阶段 1 的两项遗留:

为什么改

风险与代价(含回滚)

  • 规模:约 4,170 行变更(含 202 行 LICENSE),超过 3,000 行门槛,需要人工合并。
  • 只有显式配置了 imap.accountFor 的部署才会连接外部邮箱。仅通过 requires: ['mailbox'] 自动装配的部署不同步任何邮箱,行为与今天一致,启动时会打印一行说明。
  • IMAP 客户端是自研的最小实现,没有引入 imapflow(它带 8 个锁定版本的依赖)。主要风险是与真实邮件服务器的兼容性:
    • 已用两个独立的严格 IMAP 测试服务器(imapkit、hoodiecrow)做过一次性互通验证,18 项全部通过;
    • 尚未对 Gmail、Microsoft 365 做真机验证。
  • 附件暂不保存,转发附件里的原邮件正文也不保留,只读取其中的参与人。
  • 单封邮件超过上限(默认 32 MiB;服务器少报或不报大小时同样适用)、或无法解析时,会跳过并记 error 日志,邮件仍留在原邮箱,同步继续往下走,不会卡住。
  • 回滚:直接还原本 PR 即可。

席位意见

(终稿与席位意见见 PR 评论 6157331354。)

你要做的


Generated by Claude Code

…atcher reads it

The row was planned + authorWarn until the mailbox service's matcher existed.
The matcher (service-mailbox src/ingest/match-records.ts, MailRecordMatcher)
reads every email field declaring mailAssociation: true at ingest and at
re-match, so the author hint ("no mailbox service reads this yet") is false.
Flip to live, cite the matcher, drop authorWarn and authorHint, follow the
authorWarnedProperties('field') pin and regenerate the field state counts
(94 live / 4 planned -> 95 / 3).

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
…shaped account and credential abstraction (WIP)

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
…sor on sys_mailbox, message/global forwards (WIP)

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
…ields, changeset; connector-imap README (WIP)

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
…IMAP sync and the new package

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
…-connector-imap

# Conflicts:
#	content/docs/permissions/tenant-audit-census.mdx
#	docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
…P sync's cursor write)

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/xl dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation tests tooling labels Oct 13, 2026
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/connector-imap, @objectstack/service-mailbox, @objectstack/spec, touching 151 documentable anchor(s). ⚠️ 12 changed file(s) yielded no anchor (packages/connectors/connector-imap/LICENSE, packages/connectors/connector-imap/README.md, packages/connectors/connector-imap/package.json, …), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

38 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8.

⛔ 9 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 12 changed file(s) yielded no anchor (packages/connectors/connector-imap/LICENSE, packages/connectors/connector-imap/README.md, packages/connectors/connector-imap/package.json, …) — pages documenting those are invisible to this run
  • 2 anchor(s) matched too much of the corpus to be a work list: PERMISSION_DENIED (literal, 36 pages), organization_id (literal, 35 pages)
  • 100 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 142 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 23e3692414c9c1063cf6584435803aa9da3732a1 — the merge of head 671693ad630aadf748ee8645a596b59aaa812afa into base 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 23e3692414c9c1063cf6584435803aa9da3732a1 && git checkout 23e3692414c9c1063cf6584435803aa9da3732a1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 671693ad630aadf748ee8645a596b59aaa812afa && git checkout -B drift-repro 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 && git merge --no-ff 671693ad630aadf748ee8645a596b59aaa812afa

node scripts/docs-audit/affected-docs.mjs --json 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 27aa15166f5a21ddd5e3e4371aa1b13f6580c6f3
Local-runs: none

Read: card #22871 (body, all 13 comments), ruling 6116489638 on #22870, triage 6137138392, stage 1's 6145459294 and 6145843378, claim 6147217218, the stage-2 os-dev-report 6150880990, seat answer 6151023438, amendment 6151040021; PR #23303 body, its one comment (the docs-drift bot), its 50-file list and the net diff against main; the check-runs on the head; the head by its sha in read-only git. The API file list and git diff 175e8d414..27aa15166 agree: 50 files, +3615 / −75 = 3690. The head is 7 commits over the merge base 175e8d414: 610e112bd (the liveness flip alone), 1ddc953ae (the package), 651a2a1cc (the sync), bf2f23134, 3a8fa0423, the merge fffc37464, and 27aa15166 (the census regenerated on the merged tree).

① Derived judgments

(a) Scope against the ruled direction — holds.

  • The connector is protocol and auth only: imap-wire.ts (framing, values, astring quoting, modified UTF-7), imap-session.ts (connect, greeting, CAPABILITY, PLAIN / LOGIN / XOAUTH2, EXAMINE, UID FETCH, LOGOUT), account.ts, imap-error.ts. No parsing of mail, no dedupe, no store. Zero runtime dependencies (the manifest declares none).
  • Decision 4's shape is real: MailAccount is one identity with its own imap endpoint; MailCredentialProvider.credentialFor(account) is called inside openImapSession on every open and never cached (pinned: "resolves the credential on every open", 2 opens, 2 asks). staticCredentials is the shared mailbox's implementor; the host's accountFor resolver in MailboxServicePlugin({ imap }) is the composition.
  • Stage 1 is fed UNCHANGED. match-records.ts, mail-read-visibility.ts, the three object files other than sys_mailbox, and the rematch path have an empty diff against the merge base. mail-ingest-service.ts changes one comment line (+2 / −1). The sync calls ingest({ mailboxId, raw, receivedAt }, { isSystem: true }) and reads only outcome. Q1 A (field-level mailAssociation, no resolver), Q4 A (ingest system-only; rematch system / owner / manage_mailboxes), Q5 A (additive re-match) are untouched.
  • Q3 C phase 1: parse-message.ts now treats message/global (RFC 6532 §3.7) beside message/rfc822 as the forwarded-message part read for its headers under source attachment, through the new FORWARDED_MESSAGE_TYPES set; nested parts are still not read; inline is still written nowhere. The ruling's letter names message/rfc822 (RFC 2046 §5.2.1); message/global is that same forward-as-attachment structure with UTF-8 headers, so this is inside phase 1's structured path, not the heuristic stage. It was carried by stage 1's seat as "read it as an attachment source, or pin its refusal" and is declared in the changeset, the README and row 47b's neighbour text. Named below as a flag so the maintainer sees the letter-versus-structure reading.
  • Nothing out of scope: no per-user sync (the per-user provider is a type, not code), no CalDAV, no write-back (the session has no STORE / EXPUNGE / APPEND / SELECT path at all), no admin plane (no routes, no screens; sys_mailbox stays apiMethods: get, list), no inline-forward parsing (service-mailbox: inline-forward parsing for the Gmail, Outlook and Apple Mail "Forwarded message" blocks — the chained follow-up of #22871 (ruling Q3 C staged) #23215 stays Blocked-by: #22871).

(b) Security — the stated guarantees hold; three hardening gaps are named in ③.

  • TLS by default: security defaults to 'tls'; tls.connect is called with no rejectUnauthorized override (Node's default verifies), servername set for a host name and omitted for an IP literal (Node then checks IP SANs). No option to disable verification exists. STARTTLS is not offered, which the README states.
  • Plaintext refused except loopback, BEFORE a socket opens: isLoopback strips brackets, accepts exactly localhost, ::1, and isIP() === 4 && startsWith('127.'). Reviewed for the brief's cases: ::1 and [::1] accepted; 127.0.0.0/8 accepted; IPv4-mapped ::ffff:127.0.0.1 refused (safe side); 127.1 shorthand, 0.0.0.0, LOCALHOST, localhost. and zone-qualified addresses refused (safe side). The one soft spot is localhost by NAME: resolution is the OS resolver's, so a hosts-file or resolver compromise on the machine itself could point it elsewhere. That is a host compromise, not a network exposure; flagged, not failing. Pinned by the "refuses security none to any other host before a socket opens" test, which proves no TCP connection was made.
  • Credentials: resolved per open (above); never logged — the connector has no logger; ImapError.response carries the server's own line only; the sync logs err.message and the skip text, never the credential; the double redacts LOGIN / AUTHENTICATE lines in its commands log. Never stored on the row: the four new fields are a folder name, two integers and a timestamp; the plugin test pins "no secret or password typed field on sys_mailbox". No new secret store: staticCredentials holds a copy in a closure; the host supplies it (H2 measured all four existing connectors take a static credential the same way).
  • Read-only is a protocol fact, not a habit: examine() sends EXAMINE and the session has no SELECT; listMessages sends UID FETCH n:* (UID RFC822.SIZE); fetchMessages sends UID FETCH set (UID INTERNALDATE BODY.PEEK[]). No STORE, EXPUNGE, COPY, MOVE, APPEND or CLOSE exists in the code. A PREAUTH greeting skips authentication (safe direction).
  • Bounded reads: a response line over IMAP_MAX_LINE_BYTES (4 MiB) and a literal over maxLiteralBytes (default 64 MiB) are refused BEFORE the octets are read; the literal size is parsed from \d+ and refused unless Number.isSafeInteger, so there is no overflow path; a zero-length literal is handled. RFC822.SIZE is honoured: the sync lists sizes first and excludes anything over maxMessageBytes from the UID set it fetches (pinned: the big message is never fetched). Injection: EXAMINE takes imapAstring(encodeMailboxName(folder)) — CR, LF and 8-bit are UTF-7-encoded away, then " and \ are escaped inside a quoted string; LOGIN takes imapAstring() of both arguments — printable 7-bit is quoted with escaping, anything else goes as a synchronizing literal sent only after the server's + (pinned with a password carrying CRLF); PLAIN and XOAUTH2 payloads are base64; UID sets are built from integers only. No command is built from unescaped text.
  • What is NOT bounded (③): the number of parts per response and of untagged responses per command, and the batch's total bytes; and the session's literal cap is not tied to the sync's maxMessageBytes.

(c) The sync state — correct on every axis asked.

  • Reset: a stored cursor applies only when imap_folder === folder && imap_uid_validity === status.uidValidity; otherwise lastUid starts at 0 and the folder is read from UID 1; reset: true is reported only when a cursor existed. A folder with no UIDVALIDITY is refused by the connector (protocol), so a cursor is never taken under an untrusted UID space. Pinned: UIDVALIDITY rebuild (2 duplicates, row count 2, cursor re-stamped) and folder change.
  • Advance only after handover: lastUid = entry.uid is set after ingest resolved (stored, duplicate, suppressed), after a deliberate skip (too_large, unreadable), or when the UID vanished between list and fetch (expunged; nothing to mirror). saveCursor runs in finally per batch, so a thrown store error leaves the cursor at the last handed-over UID. Pinned: "stops at a store failure with the cursor below the failed message, and the next run retries it" (cursor 1, rows 1; then 2 more, cursor 3).
  • Crash in the middle: the worst case is a process kill between an insert and the batch's cursor write; the next run re-lists from the old cursor and every stored message answers duplicate through stage 1's unique key (mailbox_id, dedupe_key) scoped organization. Nothing is skipped; at most one batch is re-fetched.
  • The unreadable skip is safe: every VALIDATION_ERROR ingest can throw is either a caller bug the sync cannot produce (missing mailboxId; an invalid receivedAt — the sync passes a Date or undefined) or the per-message parse refusal. A transient engine error is not a VALIDATION_ERROR and stops the run instead.
  • Concurrency: one process — running map returns the in-flight promise for the same mailbox, and syncAll is serial over mailboxes, so an overlapping mailbox:imap-sync run joins the mailbox in flight and starts only mailboxes the first run has not reached. Two processes — both may fetch; the unique key makes the second insert a duplicate (stage 1 catches the duplicate-record error and re-reads the winner); a cursor regression from a slower writer only causes a re-fetch of duplicates, never a skip.
  • Organization scoping: syncAll walks every sys_mailbox row (fields: id) under the system context — necessarily, it is the process-wide job; runMailbox refuses a row with no organization_id (INTERNAL_ERROR) and hands organizationId to accountFor; the message's organization is stamped by stage 1 from the mailbox row. Census row 47c names exactly the two system-only doors (syncMailbox, syncAll) and what they get and lose.
  • Teardown: MailboxServicePlugin.destroy() cancels MAILBOX_SYNC_JOB through IJobService.cancel and calls sync.close(), which flags closed, closes every open session and makes later runs answer null; destroy() is the only hook the kernel calls (check-plugin-teardown-shape header), and the kernel test pins jobs.cancelled contains the job after kernel.shutdown(). The sync class's own close() is not a Plugin method, so it is outside that gate's roster by design.

(d) The four sys_mailbox fields — right surface, declared, translated, live.

  • All four are required: false, readonly: true, group: 'Sync', with descriptions; the plugin test pins readonly on each. readonly is enforced by the engine's stripReadonlyFields for a non-system caller and bypassed for the system context, which is exactly the contract the sync relies on ("written by the sync only" is therefore an enforced statement for every non-system door; sys_mailbox has no REST write door at all).
  • Placement: sys_mailbox's own header on main said the cursor "is added by the ingest half"; spec + service-mailbox: sys_mail_message (the inbound mirror object) and the association seam — phase 1 of #8998 inbound mail, spec seat #22870's workload line called it the identity half of one object. A separate state object would add a row per mailbox with a 1:1 join for three scalars and a timestamp; the cursor on the mailbox row is the lighter and the planned shape.
  • Liveness: imap_folder, imap_uid_validity, imap_last_uid are read in runMailbox and written in saveCursor; imap_synced_at is written in saveCursor and read by no code — it is an operator-facing status column served through the manage_mailboxes directory read, so declared-and-delivered, but write-only in code (noted).
  • Translations: en generated from the object; zh-CN, ja-JP, es-ES hand-translated with their source-hash rows; check:i18n and check:i18n-coverage reported green.

(e) The mailAssociation liveness flip — true, and the anchor resolves.

  • field.json: planned to live, verifiedAt 2026-10-13, authorWarn and authorHint dropped, evidence packages/services/service-mailbox/src/ingest/match-records.ts#MailRecordMatcher — export class MailRecordMatcher is at line 103 of that file on the head and is unchanged since 82581819b, so the row is true on main today, as stage 1's seat said. The dotted anchor the dispatch spelled is refused by check:liveness (one identifier), and the prose names match and flaggedFieldsOf; the deviation is right.
  • state-counts/field.md 94 / 4 to 95 / 3, classified 99 unchanged; the authorWarnedProperties('field') pin becomes ['conditionalRequired'] with its comment rewritten in the past tense.
  • On origin/main at 2808069121 the row is still planned, and no PR exists yet for claude/issue-22871-s2-liveness-flip (the amendment names it). Once that PR lands, merging main makes commit 610e112bd a no-op here, but this PR's changeset would still carry the @objectstack/spec: patch line and its spec bullet — see ③.

(f) Semver and changesets — see ②.

(g) The acceptance tests — each clause pinned against the double, with its control; the double is a fair stand-in.

  • mailbox-sync-service.test.ts runs on a real ObjectQL over driver-sql / better-sqlite3 with the real objects and their unique indexes and an app object whose email field declares mailAssociation: true. Clause 1: a forward-as-attachment lands as ONE row, associated, match_addresses carries the customer's address under source attachment, one sys_mail_association row matched_by: ingest, received_at equals the IMAP INTERNALDATE; a second test pins EXAMINE, no SELECT, BODY.PEEK and no \Seen. Clause 2: the second run fetches 0 and the row count stays 1; a re-delivered copy under a new UID answers duplicate; a UIDVALIDITY rebuild re-reads both as duplicates. Clause 3: an unknown sender pools (unassociated, no link); after the contact exists, rematch adds one link matched_by: rematch and moves the row to associated. Clause 4: an Auto-Submitted: auto-replied message and a multipart/report bounce store no row and the cursor moves past both; the control stores a human reply. The incremental-fetch block pins batch and run bounds, the too_large skip with its error text, the unreadable skip, and the retry-below-failure cursor. mailbox-sync-kernel.test.ts boots a real ObjectKernel with ObjectQLPlugin, a recording job service and the plugin, and pins the schedule shape, a stored-and-associated message through the job handler, degraded when the server is gone, and the cancel on shutdown.
  • The dev's 11 ablation legs map one to one onto those pins (A1 to clause 1's store, A2 / A2b / A2c to clause 2's three tests, A3 to clause 3, A4 to clause 4, A5 / A5b to the read-only pins, A6 to the loopback refusal, A7 to the two message/global parse tests, A8 to the cursor-save pins). Ablations are a report, not a diff; they are not reproducible from here and are recorded as declared.
  • The double (testing.ts, 430 lines) speaks over a real TCP socket, shares no code with the client, and exercises: server literals (every body is sent as a BODY[] {n} literal), client literals only after a + continuation, untagged FLAGS / EXISTS / RECENT / OK-coded responses, capabilities in the greeting or on request, SASL-IR and non-SASL-IR PLAIN, the XOAUTH2 error-continuation flow, LOGINDISABLED, NONEXISTENT, expunge, UIDVALIDITY rebuild with renumbering, the n:* edge, modified UTF-7 folder names, 8-bit bodies byte for byte, and INTERNALDATE with offsets. Not exercised: a multi-literal response, unsolicited untagged responses between commands, a server BYE. One inaccuracy in its self-description is in ③.
  • H1: an own double rather than imapkit (6 days old, 11 releases in 2 days) or hoodiecrow-imap (dormant 10 years, deprecated within hours of 3.x) or wildduck (MongoDB and Redis) is proportionate for a 5-command read-only subset; the one-off interop probe the dev reports is outside the tree and counts as unmeasured here. The own client (about 1,000 lines) over imapflow's 8 pinned dependencies is likewise proportionate and consistent with the package's zero-dependency claim.

(h) The surface beyond the claim — each forced by a named gate, and correct.

  • system-context.mdx row 47c (check:system-context-census): names both doors and both anchors, which exist.
  • tenant-audit-census.mdx and …counts.md (check:tenant-audit-census, tenant-audit-census.mjs --write): one new site (mailbox-sync-service.ts update MAILBOX_OBJECT, undecidable object name, elevated context), so 228 to 229, 73 to 74, 123 to 124, 167 to 168, 186 to 187, 64 to 65; the six hand-kept prose figures move consistently with the generated table; the stamp names fffc37464, the merged parent of the head, which is the tree the census was measured on.
  • llms.txt 70 to 71 (check:llms-txt: non-private @objectstack/* workspace packages) and the lockstep count 71 to 72 in backward-compatibility.mdx and publish-smoke-pack.mjs (check-lockstep-package-count.mjs: the fixed group's length) are two predicates that differed by one before and after; both are consistent.
  • The new package is in the fixed group (check-changeset-fixed); root README gains its row under the connectors; the lint pin follows the flip; the four translations follow check:i18n.
  • The lockfile: 15 added lines, two importer entries only — packages/connectors/connector-imap with three devDependencies resolved to the same specifier-and-version strings as the neighbouring importers (the vitest peer suffix is byte-identical to the entry above it, esbuild@0.28.2 included), and the link:../../connectors/connector-imap row under service-mailbox. No packages: / snapshots: entry and no integrity line was typed by hand. The hand rebuild is sound, and the head's Validate Package Dependencies and Scaffold outside the monorepo, install, build checks, which install from this lockfile, are green.

(i) LICENSE: byte-identical to connector-slack's and connector-rest's (git diff empty). The manifest's files (dist, README.md, CHANGELOG.md) matches the other connectors; npm includes LICENSE regardless.

(j) CI on the head, read three times during this review: 42 check-runs; 36 success, 5 skipped, 1 still in progress at the last read (Test Core (3/6); Test Core (4/6) and (5/6) finished green between reads). The five skips are the opt-in or roster jobs (Console Pin Gate, Dogfood Regression Gate matrix shard, Dogfood Verify CLI, Packed-tarball smoke (opt-in), Temporal Conformance (live PG + MySQL)), the same five stage 1's pre-queue read named as EXPECTED_SKIPS. No check has failed. Governed Surface Queue Guard, Part-of PR must not also close its card and The card this PR closes must claim this branch are green on the Part of body; after the body switches to Fixes #22871, the claim 6147217218 names this branch, so the third stays satisfiable. The seat's landing condition "every check green" is not yet met at this read because one shard is still running; the record is on the head, not on that shard. check:platform-checklist is deliberately not in per-PR CI (lint.yml, maintainer decision) and runs on main by the watchdog at 02:51 UTC; its last run (number 41, 287fd6e5d, 2026-10-13T03:05Z) was green, and 71 commits landed between that sha and the merge base, 8 of them touching the gate's inputs (docs/qa/platform-checklist rows re-pointed twice, metadata-protocol/src/protocol.ts changes, the security-catalog retirement), so the dev's "red on main at 175e8d414, 8 findings" is plausible and not contradicted by any measurement I can read; it is unverified here, and the next watchdog run is the measurement of record. This diff touches none of its inputs (no docs/qa/**, no metadata-protocol, no service-storage path in the 50 files).

② Semver level

  • @objectstack/connector-imap: minor — a new published package, first version 18.0.0-next.0, equal to every other member of the one fixed group in pre mode (.changeset/pre.json holds only mode and tag, so there is no initialVersions list to extend). Right: the group takes the highest bump, and a new member must enter at the group's version.
  • @objectstack/service-mailbox: minor — new exports (MailboxSyncService, MAILBOX_SYNC_JOB, four DEFAULT_SYNC_*, DEFAULT_MAILBOX_POLL_INTERVAL_MS, 11 types), a new constructor option on MailboxServicePlugin (backward compatible: the capability boot still constructs it with no arguments), four new fields on the published sys_mailbox object, and message/global added to the attachment source. Additive; minor is right.
  • @objectstack/spec: patch — a liveness ledger row and its state count, shipped in spec's files; no schema or export change. Right as a level. Duplication risk once the flip lands on its own PR: see ③.
  • Clause-②: yes on the PR body, the changeset and the claim; no (widening) / (narrowing) arm is needed and none is written. No major, no ADR-0087 disposition owed (nothing removed or renamed). check:adr-0087-registration, check-changeset-fixed, check-changeset-no-major and check-empty-changeset reported green and the head's Check Changeset is green.
  • Every changeset sentence was checked against the diff and holds: zero runtime dependencies; TLS default with the certificate verified; plaintext only to loopback, before a socket opens; EXAMINE never SELECT; BODY.PEEK never BODY; UIDVALIDITY refusal; per-open credential resolution; the ImapError kinds; the ./testing entry; the job name and interval; the cursor fields and their reset; the advance rule; maxMessageBytes 32 MiB; degraded to the job service; message/global; the export list (matches index.ts exactly); "supersedes the ingest seam entry's no mail source feeds the seam" (true: the stage-1 changeset is unreleased and unedited, and this PR's sentence is the one the compiler should carry); "attachments are still not stored" (true).
  • Fixes #22871 under ruling B: correct. Scope items: connector (this PR); folder cursor and UID state, incremental fetch (this PR); dedupe, threading, suppression, the write, the seam call, the stamp, the pool, re-match (stage 1, 82581819b); attachment extraction moved to service-mailbox: store an ingested message's attachments, and each forwarded message part as an .eml attachment, through the server-side writer: the attachment item of #22871, chained #23307, which exists, open, Blocked-by: #23306, which exists, open. Acceptance: all four clauses pinned (g). Decision 5 is a semantic the card states, not a code clause. The chained follow-up service-mailbox: inline-forward parsing for the Gmail, Outlook and Apple Mail "Forwarded message" blocks — the chained follow-up of #22871 (ruling Q3 C staged) #23215 is Blocked-by: #22871 and is unblocked by the close, as the ruling ordered. At this head the body's line 1 still reads Part of #22871; the switch is the seat's pre-landing edit and this record does not depend on it.

③ Boundary flags

The dev's declared deviations, each read:

  1. Evidence anchor #MailRecordMatcher instead of #MailRecordMatcher.match — forced by check:liveness; right.
  2. Twelve paths beyond the claim — each gate-forced (h); right.
  3. Hand-rebuilt lockfile delta — sound (h); the stated reason (pnpm's esbuild 0.28.1 / 0.28.2 peer-variant flip, the mirror of what stage 1 met) is consistent with the base lockfile keeping esbuild@0.28.2.
  4. H1 own double and own client — proportionate (g).
  5. Size measured at L (3690 lines, 202 of them LICENSE) — the subagent could not send the early status line; the Tier H route the seat chose is the consequence. The PR body's "commit 1ddc953ae is self-contained" is corrected in the report: that commit's lockfile still carries the peer flip, so a split would take the package files plus the 12-line importer entry. The body is not patched; the seat may correct it.
  6. The first gate battery's 7 reds, fixed and superseded by the merged-head battery; the check:query-options-erasure re-run — bookkeeping, consistent.
  7. Harness attribution: commits carry AGENTS.md's model-free pair — the user's instruction set wins, as the reminder's own precedence sentence says.
  8. The connector follows the connector packages' SHAPE (manifest, LICENSE, tsconfig, vitest config, co-located tests) but not their CONVENTION of a plugin registering connector_action handlers: it ships no plugin and no action. The card says "following the existing connector convention"; the PR body declares the departure and its reason (nothing in a flow calls IMAP; the mailbox service does). Reasonable, and named here because the card's wording invites the other reading. A flow-reachable IMAP action would be write-back territory, which is out of scope.

The open question, as ruled B:

Findings of this review, none failing the contract:

  1. Unbounded accumulation per response and per command against a hostile server. ImapResponseReader bounds each LINE (4 MiB) and each LITERAL (64 MiB) but not the NUMBER of parts one response may carry: a server can send an endless run of short literal-marker lines ({0} after {0}) and parts grows without bound; likewise InFlight.untagged holds every untagged response of a command with no cap, so an unlimited stream of * n FETCH lines before the tagged OK grows memory, and the socket timeout is an inactivity timeout, so a trickle keeps it alive. An IMAP server is an untrusted peer, as the brief says; a total-bytes-per-response and per-command cap (or a count cap) closes the class. Low severity (the host chose the server), owed as hardening.
  2. The sync's maxMessageBytes is not the wire bound. runMailbox opens the session without passing maxLiteralBytes, so a server that omits or understates RFC822.SIZE is bounded by the session default of 64 MiB, not by the configured 32 MiB; and a message above 64 MiB with no or a false size fails the whole run as protocol (not a skip) and would do so on every run — the one path on which "one message cannot stall a mailbox" does not hold. Passing maxLiteralBytes: maxBytes (plus a small envelope) to openImapSession aligns the two bounds; the stall needs a server that lies or omits the size.
  3. A batch is count-bounded, not byte-bounded. fetchMessages(fits) fetches up to batchSize (25) messages in one command and holds them all in memory; 25 messages near 32 MiB is about 800 MiB per batch in the worst case. A byte budget per batch is the usual fix.
  4. localhost is accepted by name, not by resolved address (b). Resolution is the OS resolver's; a spoof needs the host itself compromised. Accepting only IP literals would make the rule mechanical.
  5. The double's self-description overstates its \Seen control. testing.ts's header and the connector README say a body fetched "with BODY[] (not BODY.PEEK[]) or under SELECT" is marked \Seen; the code marks only BODY[] under SELECT (!readOnly), which is RFC-faithful (EXAMINE never changes state), so neither single mutation A5 (BODY for PEEK, still under EXAMINE) nor A5b (SELECT, still PEEK) can trip the \Seen assertion; both legs go red on the command-string regex pins instead, which is what the ablation count reports. The pins are real; the sentence about the double is not. Two words to fix.
  6. A possible duplicate release sentence. The seat is landing the liveness flip on its own PR first; after that, commit 610e112bd merges away but this changeset's @objectstack/spec: patch line and the spec bullet remain, and the compiler would carry the flip twice. When the flip PR lands with its own changeset, drop the spec line and bullet here (or keep this one and give the flip PR none).
  7. Listing cost on a large folder. listMessages(cursor + 1) returns every UID above the cursor each run, and only then does maxMessagesPerRun (500) cut; a 100,000-message folder is re-listed in full on every run until the cursor catches up. Correct, not cheap; a note for the per-user phase.
  8. imap_synced_at is written and never read in code (d). Declared and delivered as a status column; named so nobody reads it as a cursor input.

Out-of-scope findings carried from the report, each with its carrier as the seat stated:

  • check:platform-checklist red on main at the merge base: plausible, unverified here (j); the watchdog is the carrier, the seat said it checks main's CI before the queue.
  • The stage-1 changeset's "no mail source feeds the seam": superseded by this changeset's sentence; the release compiler is the carrier.
  • sys_mailbox has no administration door and the IMAP account source is code-only composition (accountFor); a deployment without server code syncs nothing and the boot says so once. A product gap for the admin-plane follow-up, not this stage's defect; no carrier today.

Implemented-by: claude/issue-22871-s2-connector-imap
Reviewed-by: session_012uFegdfR6K6uqvLzt7XsdZ

VERDICT: PASS

…r can make the session hold, and skip a lying server's oversized message

- the session refuses, as `oversized`, a response of more than 32 literals and
  more than maxCommandBytes of responses to one command (each response charged
  256 bytes beyond its own), beside the line and literal bounds;
- the sync opens the session with maxMessageBytes plus a 64 KiB framing envelope
  as its literal bound; a server that omits or understates RFC822.SIZE gets the
  batch re-read one message at a time on a fresh session, and the offending
  message is skipped as too_large instead of failing every run;
- a fetch batch is bounded by listed bytes (maxBatchBytes) as well as by count;
- the double's and the README's \\Seen sentence says what the code does;
- the changeset leaves the spec entry to the liveness-flip PR.

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
…-connector-imap

# Conflicts:
#	content/docs/permissions/tenant-audit-census.mdx
#	docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
…es with the IMAP sync's cursor write)

Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 44aabc8775cc1a16ff81f93d6233f2f74465d7fd
Local-runs: none

Delta review. Baseline: the at-tier PASS 6151327338 on 27aa15166f5a21ddd5e3e4371aa1b13f6580c6f3; the ordered round: seat note 6151344448 (five items). Read: card #22871 (body, all 14 comments — ruling record 6116501990, triage 6137138392, claim 6147217218, the stage-2 report 6150880990, seat answer 6151023438, amendment 6151040021, seat note 6151344448); PR #23303 body, both comments, the 50-file list and the net diff against main (API and git diff 404fc477e..44aabc877 agree: 50 files, +3986 / −75 = 4061); PR #23314 (head 4d4fac733, 4 files, open draft) as context for item 5; the 43 check-runs on the head; read-only git by sha. The delta is three commits over the baseline: 4488d11719f68d7410aac34f57e369c9765a33c0 (the hardening round: 15 files, +456 / −85), c33c6dc67845cf4e9cfb9c11e4e034f449f84cc9 (merge of origin/main at 404fc477e, conflicts on the two census files), 44aabc8775cc1a16ff81f93d6233f2f74465d7fd (the census regenerated on the merged tree). Net delta on the PR's own files, 27aa15166..44aabc877: 18 files, +485 / −118. npm view @objectstack/connector-imap answers E404 (never published); connector-slack has next: 18.0.0-next.0 for comparison. One read-only re-derivation of my own is named in (9) and is a reading of git blobs, not a gate run.

① Derived judgments

(1) Bounded accumulation — every path is bounded and every refusal closes the socket; one bound is loose, one ceiling is new.

  • The reader (imap-wire.ts) refuses, as oversized, a line whose pending bytes exceed IMAP_MAX_LINE_BYTES (4 MiB) before an LF arrives, a literal announced above maxLiteralBytes (at the {n} marker, before any octet is read; non-safe-integer sizes refused), and the 33rd literal marker of one response (IMAP_MAX_RESPONSE_LITERALS = 32, reset per response). The session (imap-session.ts dispatch) charges each completed untagged response parts lengths + 256 against maxCommandBytes and refuses oversized over it; the in-flight state is per command and dropped with the command's result, so many small responses across many commands never accumulate; unsolicited responses between commands are dropped (if (!command) return); the greeting waiter takes exactly one response. Every refusal routes through fail(): the waiter (greeting or in-flight command) is rejected, otherwise the failure is stored for the next run, and socket.destroy() is called — no half-open socket; chain swallows its own rejection, so no unhandled rejection. Pinned: the two hostile-double tests assert kind oversized, the message, and server.openConnections 0 afterwards; the control admits 50 honest messages under a 64 KiB command bound; the wire test pins exactly 32 admitted, 33 refused, per response.
  • Loose: the per-response bound is a COUNT. One response may carry 32 literals each up to maxLiteralBytes, and the byte bound is applied only when the response completes, so the reader can hold about 32 × (32 MiB + 64 KiB) + 33 × 4 MiB ≈ 1.2 GiB of one hostile response under the sync's configuration (≈ 2.2 GiB at the connector defaults) before maxCommandBytes (128 MiB) refuses it. Bounded, so not failing; the fix is to charge the running response against the command budget at each literal announcement. The seat's item said "bytes or count", and the count was delivered; the gap between the two bounds is what to name.
  • Time is not bounded: the socket timeout is inactivity, so a server that trickles a literal within its bound, or answers + after + to an AUTHENTICATE (the client echoes * forever), holds one run — and, since syncAll is serial, the whole pass — indefinitely. Memory stays bounded. Outside the five items; noted.
  • New ceiling: the listing UID FETCH n:* (UID RFC822.SIZE) is charged about 35 bytes + 256 per message, so 128 MiB admits roughly 450,000 messages above the cursor; a folder larger than that is refused oversized on the listing, which sits outside the isOversized handling and fails the run — and does so on every poll (60 s default), logged at warn by the plugin, with the cursor never moving. The seat's "listing cost on a large folder" note has become a hard cliff. Shared-mailbox phase 1 rarely meets it; it belongs in the README or on the per-user card. Not failing.

(2) maxLiteralBytes from maxMessageBytes — the arithmetic, the reopen and the re-read are right; the cursor does NOT move past a trailing skipped message, which the hardening round's own claim depends on. FAILING.

  • runMailbox opens every session with maxLiteralBytes = maxBytes + SYNC_LITERAL_ENVELOPE_BYTES (64 KiB) and maxCommandBytes = max(DEFAULT_IMAP_MAX_COMMAND_BYTES, max(maxBatchBytes, maxBytes) + maxBytes + 64 KiB) — 128 MiB at the defaults; a host raising maxMessageBytes to 200 MiB gets 400 MiB + 64 KiB, so one message always fits one command. Pinned exactly for the literal bound, and as "≥ the default" for the command bound.
  • The re-read: on an oversized batch fetch nothing was ingested (ingest runs only after a resolved fetch), reopen() releases the failed session, connects a fresh one, re-examines and refuses with protocol when UIDVALIDITY moved — that throw escapes the batch, the per-batch finally saves the cursor at the last judged UID under the OLD validity, the next run reads the new validity and resets from UID 1 (every stored message answers duplicate), which is correct. The isolated loop fetches one UID at a time, at most one reopen per entry, so it terminates; stage 1's unique (mailbox_id, dedupe_key) holds; the cursor never passes an unjudged UID. A message within the envelope but above maxBytes once read is skipped too (pinned). The skip log names UID, folder, mailbox id, the listed and actual sizes, the consequence and the fix; the session is read-only, so the message stays in the folder. All of that holds.
  • What does not hold: all three too_large paths continue past result.lastUid = entry.uid (mailbox-sync-service.ts lines 364–366, 371–385, 387–398); only unreadable and an expunged UID fall through to it. A skipped message that is the last judged in its batch — the newest message of a quiet mailbox, the common case — is therefore never passed: the next run lists it again, skips it again, logs the same error again, every poll (60 s default) until a later UID arrives; on the lying-server path each run also aborts two connections mid-FETCH (oversized, reopen, oversized, reopen). The module header ("the cursor moves … to the highest UID whose message was … deliberately skipped"), the README ("logged at error, once"), the changeset ("the cursor moves after each batch past every message … skipped") and the error text itself ("The sync moves past it … nothing else will report it") all say otherwise. Prime Directive chore: version packages #10: declared, not delivered. The listed-size path had this shape at 27aa15166 — the prior record's (c) "lastUid = entry.uid is set … after a deliberate skip (too_large, unreadable)" was wrong for too_large — and the hardening round built its two new paths on it, so the round's own deliverable ("skipped, not stalled", "moves the cursor past it") is half delivered. Every pin places the offender FIRST with a good message after it (lastUid: 2 is reached through the second message), so the trailing case is unpinned; a pin with the offender last — second run fetched 0, skipped [], imap_last_uid past it — goes red on this code. Fix: advance result.lastUid on every skip (set it inside skip() or drop the three continues) and add that pin. Nothing else in this section needs to move.

(3) The batch byte budget — right. batchesOf: budget = max(maxBatchBytes ?? 64 MiB, maxBytes), never below one message; cost is maxBytes for an unsized entry, 0 for one listed above the bound (it is kept in the batch for its skip but filtered out of fits, so never fetched), its size otherwise; a batch closes when the next entry would exceed batchSize or budget, and a single entry always fits alone. maxBatchBytes is threaded from MailboxServicePlugin({ imap }). Pinned: four equal messages under a two-message budget give UID FETCH 1:2 then 3:4; the control gives one 1:4.

(4) The \Seen sentence — true to the code and to RFC 3501. The double marks \Seen only for BODY[] when !conn.selected.readOnly (line 388), and readOnly is name === 'EXAMINE' (line 358). RFC 3501 §6.4.5: BODY[...] implicitly sets \Seen, BODY.PEEK[...] does not; §6.3.2: an EXAMINEd mailbox permits no change to its permanent state. testing.ts's header and the connector README now say exactly that and add what the two single mutations A5 / A5b prove (read-only by the commands sent). Right.

(5) The changeset and the split-off flip — the release is right in either order; one sentence of the flip differs; the spec line's obligation rides on #23314.

(6) The published-API change inside the PR — sound. ImapErrorKind is a closed string-literal union; oversized is its seventh member, documented in imap-error.ts, the README and the changeset. Moving the line and literal refusals from protocol to oversized makes the four bound refusals one kind and leaves protocol meaning "cannot read", which the sync's isOversized (the only in-tree kind reader) relies on. The package has never been published (npm E404; it is absent from the next tag the group carries), so no consumer exists and no ADR-0087 entry is owed. New exports DEFAULT_IMAP_MAX_COMMAND_BYTES, IMAP_RESPONSE_OVERHEAD_BYTES, IMAP_MAX_RESPONSE_LITERALS, the option maxCommandBytes; in service-mailbox the exports DEFAULT_SYNC_MAX_BATCH_BYTES, SYNC_LITERAL_ENVELOPE_BYTES and the option maxBatchBytes, each mirrored in sync/index.ts, index.ts and the changeset's export list (checked against index.ts). Nit: the changeset's first bullet still spells openImapSession({ account, credentials, timeoutMs?, maxLiteralBytes? }) without maxCommandBytes?; the README has it.

(7) The merge and the census regeneration — clean, exact, nothing stale. git diff 404fc477e..c33c6dc67 names only PR files, so nothing outside the PR differs from main's side. Inside the PR's files the merge changed three: the two census files (resolved to main's side: 225 sites at 66949b772, the sync's row gone) and system-context.mdx, where main (#23302) removed its own row 11b — row 47c persists. The generator and checker are byte-identical to main's. The regeneration adds exactly one row — mailbox-sync-service.ts update MAILBOX_OBJECT, undecidable, elevated — and the figures that follow from one elevated, undecidable, readable-engine site: 225 → 226, undecidable 72 → 73, threading 164 → 165, elevated 120 → 121, readable-engine 185 → 186, run-time expression 63 → 64; six prose figures re-applied (53 of the 226, **226**, 153 of 226 … 73, 121 … 0 … 97, 121 of 226 (54%), 2 / 226); stamp c33c6dc67, the tree it was measured on; corpus scale +2 sources, +1 engine-shaped type (MailSyncEngine), not enforced. Net against current main (cc14657) is that one row plus those figures and nothing else. The merge dropped or altered no code the prior record relied on: git diff 27aa15166..44aabc877 is empty on account.ts, the connector manifest and configs, LICENSE, objects/, ingest/, the plugin and kernel tests, the translations, packages/spec/liveness, llms.txt, packages/lint, the lockfile, the lockstep pair and .changeset/config.json.

(8) Nothing the prior PASS relied on is undone. The hardening diff on imap-session.ts touches only the bound constants, the option, the InFlight charge and the constructor; isLoopback, tlsConnect with Node's default verification, EXAMINE, BODY.PEEK[], the absence of any STORE / EXPUNGE / SELECT path, per-open credentialFor are unchanged. The four acceptance clauses' tests are unchanged (the test diff only adds); the sys_mailbox object file and its readonly pins are unchanged; zero runtime dependencies (manifest unchanged); the per-user-shaped MailAccount / MailCredentialProvider unchanged. Only the prior record's reading of the too_large advance (2) was never true.

(9) CI on the head: no non-green check. 43 check-runs, 38 success, 5 skipped, 0 failures, 0 in progress. The five skips are the roster five (Console Pin Gate, Dogfood Regression Gate matrix shard, Dogfood Verify CLI, Packed-tarball smoke (opt-in), Temporal Conformance (live PG + MySQL)). All five required contexts are green. check:platform-checklist is not in per-PR CI; the watchdog's last run (#41, 287fd6e5d, 03:05 UTC today) was green and none has run since, so the "8 findings at 175e8d414" remain the dev's measurement. One commit in the merge window, 543b33dee (#23290), changed packages/metadata-protocol/src/protocol.ts, an input of the gate, so input-invariance alone does not carry "the same 8" to this head. A read-only re-derivation (a script of mine reading git blobs by sha, not the gate) of the areas' anchors into the two named files finds the same unresolved set at 175e8d414 and at 44aabc877: anonymousFormIntakeOrgScopeRefusal, anonymousFormIntakeReopenRefusal, envWideRawViewRows (access-security.json) and countable (search.json), six occurrences, all into protocol.ts; the attachment-access-hooks.ts anchors resolve by text, and the 27-below-28 floor is not reproducible by text — the gate's grammar is stricter, so this is consistent with the same 8 and is not a verification of them. This PR's 50 files include none of the gate's inputs (no docs/qa/**, no metadata-protocol, no service-storage), and the delta adds none. The next watchdog run (02:51 UTC) is the measurement of record.

② Semver level

  • @objectstack/connector-imap: minor — a new, unpublished package entering the one fixed group at 18.0.0-next.N; the ImapErrorKind move is not breaking because no published consumer exists. Right.
  • @objectstack/service-mailbox: minor — two new exported constants, one new plugin and sync option, the changed oversized handling; additive. Right. The export list in the changeset equals index.ts.
  • @objectstack/spec — no line here; patch on fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314 (its own Clause-②: no). Right as a level, with the ordering dependency in (5).
  • Clause-②: yes, no arm, on the changeset; check:adr-0087-registration, check-changeset-fixed, check-changeset-no-major, check-empty-changeset and the head's Check Changeset are green. The changeset's cursor sentence is the one sentence that is not true of the code (2).

③ Boundary flags

The dev's deviations, each read:

  1. llms.txt 70 → 71 and the lockstep 71 → 72 without a spec changeset line — right: both are counts of the group, not spec changes; the spec line moved to fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314 (5).
  2. The census re-conflict — the pair conflicted again on this merge (as the dev predicted) and was resolved by taking main's side and regenerating in a separate commit, which is the os-regen-merge discipline; the regeneration is exact (7). It will conflict once more on every further merge of main that moves the count, and this PR lands by human merge, so the regeneration must be redone on the final head; the queue leg's check:tenant-audit-census is what catches a stale count on main afterwards.
  3. The ImapError kind move — coherent and unpublished (6); the changeset's openImapSession signature line lags the README by maxCommandBytes?.
  4. The seat's "noted, not changed" items stand as noted: localhost by name (host compromise only); imap_synced_at written and never read in code; listing cost on a large folder — now with a cliff at about 450,000 messages above the cursor (1), which is the one of the three that changed character in this round.
  5. reopen() connects a fresh session even after close() set closed (the batch loop checks closed only at its start, the isolated loop never); the run's outer finally releases it, so nothing leaks past the run. Minor.
  6. The PR body (Part of, the liveness clause, the "Why Part of" paragraph) and the claim line predate the seat's ruling and the changeset edit; the seat's edit, not the dev's (5).
  7. The flip's one-sentence divergence from fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314 (5): a one-line conflict for whichever lands second, not a silent drop.

Findings of this review:

  1. FAILING — the cursor does not move past a trailing too_large skip (2): three continues bypass the advance; the header, README, changeset and error text say the opposite; no pin covers the offender-last case. Fix in mailbox-sync-service.ts (advance on every skip) plus one pin (offender last; second run fetches 0 and skips nothing). The listed-size path inherited it from 27aa15166; the two new paths copy it.
  2. The per-response count bound leaves about 1.2 GiB of one hostile response holdable before the byte bound fires (1). Hardening, not failing.
  3. Unbounded time under a trickling or +-looping server; serial syncAll makes one such mailbox block the pass (1). Note for the per-user phase.
  4. The listing refusal ceiling at about 450,000 messages above the cursor (1): a permanent per-poll failure on such a folder; name it or carry it.

Out-of-scope, carried as the seat stated: check:platform-checklist red on main — plausible and consistent under the read-only re-derivation, measured by the next watchdog run (9); the stage-1 changeset's superseded sentence — the release compiler; sys_mailbox has no administration door — the admin-plane follow-up.

Implemented-by: claude/issue-22871-s2-connector-imap
Reviewed-by: session_012uFegdfR6K6uqvLzt7XsdZ

VERDICT: FAIL

… in its batch too

All three too_large paths (listed above the bound, refused by the session
when fetched alone, above the bound once read) skipped past the cursor
advance, so a skipped message that was the last judged in its batch was
listed, skipped and logged again on every poll until a later UID arrived.
skip() now advances the cursor itself. One pin per path places the
offender last: the first run skips it, the second fetches nothing, skips
nothing, and imap_last_uid is past it.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
…nd bound at each literal announcement

The per-command byte bound was applied only when a response completed,
so one response could hold up to 32 literals of maxLiteralBytes each
before it fired. The reader now asks the session at each literal's
announcement, before any octet is read, and the session refuses
`oversized` when the command's kept bytes plus the running response
would pass maxCommandBytes. The double gains a `withheld-literal`
hostile mode (the second literal is announced, never sent); the pin
asserts the refusal kind, its message and the closed connection.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
…uncement charge, and the listing ceiling

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
#23314 landed the mailAssociation liveness flip as 777eed2 with the
trimmed note; the one conflicting line, that row's note in
packages/spec/liveness/field.json, is resolved to main's side, so this
branch's flip commit is a no-op against main.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012uFegdfR6K6uqvLzt7XsdZ
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 671693ad630aadf748ee8645a596b59aaa812afa
Local-runs: none

Delta review, round 3. Baseline: the at-tier PASS 6151327338 on 27aa15166f5a21ddd5e3e4371aa1b13f6580c6f3 and the at-tier FAIL 6154709411 on 44aabc8775cc1a16ff81f93d6233f2f74465d7fd; the ordered round: seat note 6154727825 (four items; unbounded time carried). Read: card #22871 (body, all 17 comments), PR #23303 body (not evidence; mid-edit by the seat), its 3 comments (the docs-drift bot and both prior records), the 47-file list and the net diff against main (the API diff and git diff 430e5f5ad..671693ad6 agree: 47 files, +4105 / −63 = 4168), the 43 check-runs on the head and the step list of the one job whose census step is continue-on-error; read-only git by 40-hex sha only. The delta over 44aabc877 is four first-parent commits: a11f7c57e2f42974efd6336e982a300f201904b3 (the cursor fix, +3 in the sync, +69 of pins), 2725aacca1f6e239d8595183bb3292a01b0b4a54 (the per-announcement charge: session +19 / −2, wire +18 / −2, double +21 / −4, one pin), 5e075ec2f884db6575f3dece485c193f65c3dec6 (changeset and the two READMEs, +3 / −2) and 671693ad630aadf748ee8645a596b59aaa812afa (the merge of origin/main at 430e5f5ad128792a5f0e4acfbbd14ab79cd317e8, which carries 777eed2bc4d8e44afcd7057fdfc239a9a25760c3, the landed #23314). Net on the PR's own package sources, 44aabc877..671693ad6: 9 files, +139 / −10, exactly those three commits; main at bbb975771781c0d75e0a367b956661c3745c2788 is 5 commits past the merge base and none of the 22 files it moved is among the PR's 47.

① Derived judgments

(1) The failing defect — fixed on every skip path, pinned offender-last on every too_large path, monotonic; the ablation count follows. PASSES.

  • skip() (mailbox-sync-service.ts lines 458–478) now sets result.lastUid = uid after pushing the skip record, and every one of its four callers passes the loop's own entry.uid: the listed-size skip (line 365), the isolated oversized skip (374), the post-read length skip (388) and the unreadable skip (411). The three too_large paths still continue past the loop's tail assignment (415), so each passes the cursor exactly once, inside skip(); the unreadable path falls through to the tail and assigns the same value a second time, idempotently (named in ③, no effect).
  • Never backward, never past an unjudged UID. listMessages returns a Map keyed by UID, filtered to uid ≥ from, sorted ascending (imap-session.ts 467–482); taken is a prefix slice of that; batchesOf consumes taken in order into consecutive slices; the loop walks batch in that order, both in the batched case (the fetched Map is keyed by UID, so the server's response order is irrelevant) and in the isolated re-read (each entry fetched alone, in the same order). A skip of a lower UID after a higher one was judged is therefore impossible within a run, and every entry before the current one was stored, duplicated, suppressed, skipped or found expunged, or the loop would have thrown first. When the batch fetch is refused oversized, fetched stays the empty Map it was initialised as (the assignment threw), so no partial result from the refused command is ever read.
  • The per-batch finally save (417–419) is still right and is improved by the fix: a skip followed by a throw (the isolated fetch of the next entry failing with a non-oversized error, or reopen() throwing protocol on a UIDVALIDITY move) now saves the cursor past the skipped UID and below the failed one, which is what both messages deserve. When the batch fetch's reopen() throws before the inner try, no save runs for that batch, and nothing in it was judged, so none is owed. A refused oversized skip under the old validity saves under status.uidValidity; the next run reads the new validity and resets, as before.
  • The four offender-last pins (mailbox-sync-service.test.ts 356–423) each place a good message first and the offender second under maxMessageBytes: 1024, run the sync twice, and assert: first run fetched: 1, stored: 1, lastUid: 2 with one too_large skip of UID 2 carrying the path's own detail text; second run fetched: 0, skipped: [], lastUid: 2; zero BODY.PEEK commands in the second run (peeks() difference); imap_last_uid 2 on the row; errors of length 1 across both runs (the array is emptied in beforeEach, and lying() replaces the double but not the array). The three paths are covered as ordered: listed above the bound (a 4096-byte body, listed truthfully); refused by the session when fetched alone, once with sizeReport: 'understated' and once with 'omitted' (a 100,000-byte body against a literal bound of 1024 + 64 KiB, so the batch fetch is refused at the literal's announcement, the session reopens, the first message lands alone, the second is refused alone and skipped); above the bound once read, inside the framing envelope (a 4000-byte body understated to 100, read whole, then skipped by its length). The second run's listing UID FETCH 3:* answers UID 2 under the n:* rule and the filter keeping only UIDs above lastUid drops it, which is why fetched is 0 without a fetch command.
  • Ablation F1 (remove the advance inside skip()): exactly the four new pins go red, because every older too_large pin places the offender first and reaches lastUid: 2 through the good message after it, and the unreadable pin is carried by the loop's tail assignment. Four red is what the tree predicts; it is reported, not re-run here.
  • The sentences the FAIL named as false are now true of the code: the module header ("deliberately skipped"), the README ("logged at error, once"), the changeset ("past every message … skipped") and the error text ("The sync moves past it … nothing else will report it").

(2) The per-announcement charge — one hostile response is bounded by maxCommandBytes plus one line; no double charge; no honest refusal; the double's new mode is a fair, additive surface.

  • The reader (imap-wire.ts) keeps responseBytes (each line's length and each literal's octets of the response in progress) and, at a {n} marker, after the count cap (32) and the per-literal cap, calls admitLiteral(responseBytes + size) before literalRemaining is set, so before any octet of the announced literal is read (line 106). The session's callback (imap-session.ts 280–288) refuses oversized when current.keptBytes + 256 + responseBytes + size exceeds maxCommandBytes. keptBytes holds only completed untagged responses (344–346), so the running response is charged once at each announcement and once more, in full, on completion; the two charges use the same measure (string parts by .length, literal parts by byte length) and the same strict comparison, so a response admitted at its last announcement completes at most one trailing line heavier (the closing paren) than it was judged at. Between announcements the reader holds at most one pending line, itself bounded by IMAP_MAX_LINE_BYTES (4 MiB), so what one command can hold is maxCommandBytes plus one line, as the header now says. The throw leaves reader.push inside onData's try, reaches fail(), rejects the in-flight command and destroys the socket, exactly as the other three refusals do; no unhandled rejection, since chain swallows its own.
  • No honest refusal. The listing carries no literal, so it is charged as before. A batch fetch's responses cost their bodies plus about 65 bytes of line and 256 of overhead each; under the sync's bound, max(128 MiB, max(maxBatchBytes, maxMessageBytes) + maxMessageBytes + 64 KiB), a batch of exactly maxBatchBytes listed bytes from a server whose RFC822.SIZE is honest costs at most maxBatchBytes plus batchSize × about 321 bytes, under the bound by at least maxMessageBytes + 64 KiB minus 8 KiB at the defaults, and batchSize is bounded by maxMessagesPerRun (500) in practice. The 50-message control (unchanged, green on this head) lists 50 and fetches 3 under a 64 KiB bound, about 16 KiB kept. String lengths count UTF-16 units, so a multibyte line is under-charged, the lenient direction, and both charges agree (③).
  • The pin (imap-session.test.ts 249–261): withheld-literal under a 64 KiB bound announces and sends a 40 KiB literal, then announces a second 40 KiB literal and sends nothing; the first announcement is admitted at about 41.2 KiB and the second refused at about 82.2 KiB; the pin asserts kind oversized, the announcement message naming 65536, and zero open connections 20 ms later. Without the charge the session would sit until its 5 s silence bound; the test's own 10 s budget would then still pass only on timeout, so the kind assertion is what carries it.
  • The double's mode (testing.ts 380–385): a third member of the hostile union on the ./testing entry, checked before the two endless modes, so neither of them moves; it ignores the asked UID set and always announces UID 1, and announces the second literal as BODY[HEADER], a legal FETCH item shape, so the client's parser is not what refuses it. The package has never been published (npm view answered E404 at round 2; the manifest still says 18.0.0-next.0), the entry is a declared test double, and the widening is additive. Acceptable.

(3) Changeset and READMEs — every changed sentence true; one formula is a simplification in the safe direction.

  • The changeset's openImapSession line now reads { account, credentials, timeoutMs?, maxLiteralBytes?, maxCommandBytes? }, which is ImapSessionOptions (imap-session.ts 54–63) exactly. The nit from round 2 is closed.
  • The connector README's bounds sentence adds "and the response being read charged at each literal's announcement, before its octets are read": true (2).
  • The service README's new bullet: the listing is one command (true: listMessages is one UID FETCH n:* (UID RFC822.SIZE)), kept under the session's maxCommandBytes, "which the sync sets to the larger of 128 MiB and maxBatchBytes + maxMessageBytes + 64 KiB" — the code (mailbox-sync-service.ts 279, 287) is max(128 MiB, max(maxBatchBytes, maxMessageBytes) + maxMessageBytes + 64 KiB); the two differ only when a host sets maxBatchBytes below maxMessageBytes, where the code's bound is the larger, so the README never overstates the bound. "About 300 bytes per message listed": a listing line of about 45 bytes plus the 256 overhead, right. "More than about 450,000 messages above the cursor is refused oversized on the listing, on every poll": 128 MiB over 300 is about 447,000; the listing sits before the batch loop and outside the one isOversized catch, so the refusal escapes the run with the cursor unmoved, and the plugin's job reports it on every poll, right. "Until the host raises that bound through maxBatchBytes": true as a remedy, since maxBatchBytes above about 96 MiB lifts the bound one for one; the dev's declared deviation from the seat's wording (no maxCommandBytes sync option) is read in ③. "Paging the listing is left to the per-user phase": a scope statement, consistent with the carried finding.
  • No other sentence changed in this round. The changeset's Clause-②: yes line, its export list (checked against service-mailbox/src/index.ts, which the round did not touch) and its cursor sentence hold.

(4) The merge 671693ad6 — the one conflict resolved to main's side, the flip a no-op, the census neither conflicted nor stale, the PR's sources untouched; no spec line owed.

  • git diff 430e5f5ad 671693ad6 -- packages/spec/liveness packages/lint/src/lint-liveness-properties.test.ts is empty, so 610e112bd is a no-op against main, and the three files have left the PR's file list (50 at round 2, 47 now). git diff 5e075ec2f 671693ad6 -- packages/spec/liveness/field.json shows exactly one hunk: the mailAssociation row's note, the PR's wording replaced by the trimmed sentence fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314 landed; status, verifiedAt and evidence are identical on both sides. The merge message says the same.
  • The census pair did not conflict: git diff 404fc477e 430e5f5ad is empty on both census files and on the generator and checker, so main had not moved them since the previous merge; git diff 5e075ec2f 671693ad6 on the pair is empty too. Against main the pair carries the one row and the figures round 2 verified (225 → 226 and its six derived figures; stamp c33c6dc67, the tree it was measured on; the gate reads the stamp's presence, not its value). The gate is green on this head by its own step: in Lint · repo & tooling gates the step "Tenant-audit census matches the tree" is continue-on-error: true by the file's design, so I read the job's 137 steps rather than its conclusion — step 66 is success, and the "Lane verdict" step that reads every gate's outcome is success. main's five later commits do not touch the pair, so the count holds against bbb975771 as well.
  • The PR's own package sources are byte-identical between 5e075ec2f and 671693ad6: the diff over packages/connectors/connector-imap, packages/services/service-mailbox, the changeset, .changeset/config.json, the lockstep pair and backward-compatibility.mdx is empty. Of the PR's shared files, only content/docs/permissions/system-context.mdx moved, on main's side (rows 11 and 55b reworded for ADR-0131 D3, one paragraph on the org-admin grant); row 47c (syncMailbox, syncAll) is intact at line 173. The root README, pnpm-lock.yaml and llms.txt did not move. What main brought outside the PR's files (132 files) is main's own.
  • The spec changeset line: .changeset/22871-mail-association-live.md ('@objectstack/spec': patch, Clause-②: no) is on main at 430e5f5ad and unconsumed, so the flip's sentence ships once, from fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314, and this PR's changeset rightly carries no spec line for it. The llms.txt count 70 → 71 does ship in spec's tarball (llms.txt is in spec's files), but it is a derived workspace figure held by check:llms-txt against the workspace, moved by the new package and not by any spec change; the new package's own minor entry is the sentence an upgrading reader needs, spec moves with the fixed group whatever its CHANGELOG says, and no gate asked for the line at 44aabc877 (Check Changeset green there and here). Not owed; named in ③ so the maintainer sees it.

(5) Nothing the two prior records relied on is undone. The fix round touches nine files, and none of account.ts, imap-error.ts, the connector index.ts, the manifest, LICENSE, the sync index.ts, the service index.ts, objects/, ingest/, the plugin and kernel tests, the translations, the lockfile or the lockstep pair. On the head: examine() sends EXAMINE and the session has no SELECT, STORE, EXPUNGE, COPY, MOVE, APPEND or CLOSE path; fetchMessages sends BODY.PEEK[]; security defaults to 'tls' and tlsConnect runs with Node's default verification (no rejectUnauthorized override anywhere); security: 'none' is refused unless isLoopback before any socket opens; credentialFor is asked per open; the manifest declares no dependencies and no peerDependencies; the four sys_mailbox fields are readonly: true under apiMethods: ['get', 'list']; the four acceptance clauses' tests are byte-identical to round 2 (the test diff only adds a describe inside "the incremental fetch"); the line, literal, literal-count and command bounds of round 2 stand and every refusal, the new one included, routes through fail() to socket.destroy(), with the two hostile pins and the control unchanged beside the new one. The ImapResponseReader constructor gained an optional admitLiteral, and the reader is not exported from the package's entry, so no public signature moved.

(6) CI on the head — no non-green check. 43 check-runs, all on 671693ad63: 38 success, 5 skipped, 0 failures, 0 in progress. The five skips are the roster five: Console Pin Gate, the Dogfood Regression Gate matrix shard, Dogfood Verify CLI, Packed-tarball smoke (opt-in), Temporal Conformance (live PG + MySQL). The five required contexts are success: Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core, Governed Surface Queue Guard. Check Changeset, Check PR Size, Spec property liveness, the three card guards and Scaffold outside the monorepo, install, build are success. mergeable_state is clean against a main five commits past the merge base, with no file overlap. check:platform-checklist is not in per-PR CI; this round adds none of its inputs, and the watchdog run on main remains the measurement of record, as both prior records said.

② Semver level

  • @objectstack/connector-imap: minor — unchanged from round 2: a new, unpublished package entering the one fixed group at 18.0.0-next.N. This round adds no export (index.ts untouched; ImapSessionOptions already carried maxCommandBytes?) and widens one union on the ./testing entry's options by a third literal. Right.
  • @objectstack/service-mailbox: minor — unchanged: the round changes behaviour inside skip() toward what the changeset already promised and adds no export. The export list in the changeset still equals index.ts. Right.
  • @objectstack/spec — no line here; its patch shipped with fix(spec): the mailAssociation liveness row goes live — the mailbox matcher reads it, so the stale author warning is dropped #23314 (777eed2bc) and sits unconsumed on main. Right, with the llms.txt reading in (4).
  • Clause-②: yes, no arm; no major, no ADR-0087 disposition owed; Check Changeset green on the head.

③ Boundary flags

The dev's declared deviations, each read:

  1. Gates run outside the lock, as the agent file allows. A report, not a diff: this record's Local-runs is none, and the dev's local build, suites, ablations and gate battery are recorded as declared, with the head's 43 check-runs as the measurement I can read. The one ablation this round depends on (F1, 4 red) is consistent with the tree (1).
  2. The README remedy via maxBatchBytes, not a maxCommandBytes sync option. True to the code and sufficient as a remedy (3); the cost is coupling: a host that only wants a larger listing bound also raises the per-batch fetch budget, and so the memory one batch may hold, which the README does not say. A dedicated sync option, or paging the listing, belongs with the per-user card; nothing here forecloses either.
  3. The withheld-literal testing mode — additive on an unpublished package's declared double; proportionate to the pin it serves (2).
  4. The carried unbounded-time finding (round 2, finding 3): a trickling or +-looping server still holds one run, and serial syncAll one pass, within memory bounds; the seat carried it to the per-user phase with no fix here. It stands as carried; this round narrows nothing about it and widens nothing.

Findings of this review, none failing:

  1. The service README's formula names maxBatchBytes where the code takes max(maxBatchBytes, maxMessageBytes); the two agree on every configuration where maxBatchBytes is at least maxMessageBytes, which is every sane one, and the README understates rather than overstates the bound otherwise (3). One phrase, if the seat wants it exact.
  2. The announcement and completion charges measure text parts in UTF-16 units, so a line with multibyte octets is under-charged by both, consistently and in the lenient direction; the bound is a ceiling against hostility, not an accounting of bytes, so this costs nothing today (2).
  3. On the isolated re-read, too_large is the verdict for any oversized refusal of one message's fetch, not only a literal above the bound: a server whose one FETCH response for a UID carries a line above 4 MiB or more than 32 literals has that message skipped, logged once naming the lie, and passed, which is the designed outcome for a message the server cannot deliver sanely; named so nobody reads too_large as "size" alone.
  4. unreadable assigns lastUid twice, in skip() and at the loop's tail, to the same value; harmless, and the three continues are what make the tail unreachable for the others. Dropping the tail assignment for the skip case would make "exactly once" literal; not owed.
  5. The withheld-literal double always announces UID 1 whatever set is asked; fine for a hostile mode, and the pin asks for UID 1.

Carried as the seat and the prior records stated: localhost accepted by name (host compromise only); imap_synced_at written and never read in code; the listing cost on a large folder, now named with its ceiling and remedy in the README; reopen() after close() (released by the run's outer finally); check:platform-checklist red on main (the watchdog is the carrier; no input of it in this diff); the stage-1 changeset's superseded sentence (the release compiler); sys_mailbox with no administration door (the admin-plane follow-up); the llms.txt count shipping in spec's tarball without a spec sentence (4).

Landing shape, unchanged: 47 files, +4105 / −63 = 4168 changed lines against main, over the 3,000-line threshold, so this PR lands the way the seat answer 6151023438 set — by human merge, with no seat arming auto-merge; the diff touches no governed surface. The census pair will conflict again only if main moves the count before the merge, and main has not since 430e5f5ad.

Implemented-by: claude/issue-22871-s2-connector-imap
Reviewed-by: session_012uFegdfR6K6uqvLzt7XsdZ

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

维护者速读 · PR #23303(671693ad6)· domain:spec 席 3 · 2026-10-13T23:24Z

改了什么

  • 新增发布包 @objectstack/connector-imap。它只做协议和认证:以只读方式连接一个 IMAP 邮箱,读出邮件原文。用 EXAMINE 打开文件夹,用 BODY.PEEK 读取,所以不会把邮件标成已读,也不改动邮箱。
    • 支持密码(PLAIN / LOGIN)和 OAuth 令牌(XOAUTH2)。
    • 默认 TLS 并校验证书。明文连接只允许连本机。
    • 对外部服务器设了内存上限:单行、单个邮件体、单个响应、单条命令的累计响应,超过就拒绝并断开。一个恶意或出错的服务器拖不垮本进程。
    • 账号与凭据按"每个用户一个邮箱"的形状设计,共享邮箱是第一个使用者。以后接个人邮箱是扩展,不是重写。
    • 无任何运行时依赖。
  • @objectstack/service-mailbox 新增邮箱同步。
    • 部署方组合插件时提供"哪个邮箱用哪个 IMAP 账号和凭据"(imap.accountFor),插件把同步注册成定时任务,定期把新邮件交给阶段 1 已上线的接收入口。
    • 去重、线程、按邮箱字段自动关联到客户/线索记录、未匹配进池、自动回复与退信过滤,全部沿用阶段 1,没有改动。
  • sys_mailbox 新增 4 个只读字段,记录同步进度:文件夹、UIDVALIDITY、最后同步的 UID、最后同步时间。凭据不存数据库。
  • 顺带完成阶段 1 的两项遗留:

为什么改

风险与代价(含回滚)

  • 规模:约 4,170 行变更(含 202 行 LICENSE),超过 3,000 行门槛,需要人工合并。
  • 只有显式配置了 imap.accountFor 的部署才会连接外部邮箱。仅通过 requires: ['mailbox'] 自动装配的部署不同步任何邮箱,行为与今天一致,启动时会打印一行说明。
  • IMAP 客户端是自研的最小实现,没有引入 imapflow(它带 8 个锁定版本的依赖)。主要风险是与真实邮件服务器的兼容性:
    • 已用两个独立的严格 IMAP 测试服务器(imapkit、hoodiecrow)做过一次性互通验证,18 项全部通过;
    • 尚未对 Gmail、Microsoft 365 做真机验证。
  • 附件暂不保存,转发附件里的原邮件正文也不保留,只读取其中的参与人。
  • 单封邮件超过上限(默认 32 MiB;服务器少报或不报大小时同样适用)、或无法解析时,会跳过并记 error 日志,邮件仍留在原邮箱,同步继续往下走,不会卡住。
  • 回滚:直接还原本 PR 即可。

席位意见

  • 建议批准。 契约评审走了三轮:第一轮 PASS(6151327338);加固轮后增量评审 FAIL(6154709411);修复后 PASS(6157310691,head 671693ad6)。
    • FAIL 的问题是:被跳过的超大邮件若是批次最后一封,游标不越过它,每次轮询都会重复跳过。现已修好,四条跳过路径都补了"超大邮件排最后"的测试,并做了消融验证。
    • 恶意服务器单个响应的内存占用,现在封顶在命令字节上限之内。
  • 当前状态: CI 全绿(38 项成功,5 项按名单跳过);不碰任何受管面;与 main 无冲突。
  • 已知未解决(不阻塞):
    • 遇到慢速滴流的服务器时,单次同步没有总时长上限(内存仍有界),记入"每用户一个邮箱"阶段;
    • 尚未对 Gmail、Microsoft 365 做真机验证。
  • 附件按裁决 B 拆到 service-mailbox: store an ingested message's attachments, and each forwarded message part as an .eml attachment, through the server-side writer: the attachment item of #22871, chained #23307,这一点你在轮次报告里仍有否决权。

你要做的


Generated by Claude Code

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

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

3 participants