Repository navigation
test(script-bodies): the send_email pin reads the sys_email insert the engine answered with its own id, and the test boot's email service is built the way objectstack serve builds it - #2177
Conversation
…e engine answered with the body's id, not the window's first The pin took the FIRST sys_email insert its engine recorder saw while send_email ran. The recorder's window is time, not cause: the messaging dispatcher sends an earlier write's notification emails on its own loop, so another writer's insert can land ahead of the body's. Measured: the contact_welcome email of a contact created earlier in the file, refused by the email service (no from address on this boot) and recorded as a failed row on each retry, which is the `expected 'failed' to be 'queued'` CI failure. The pin now awaits each recorded insert's answer and reads the one the engine answered with the id send_email returned, asserting there is exactly one. Every other claim of the pin is unchanged. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDgipbNowaGa139em8Fhoq
…e argument objectstack serve gives it
The harness built `new EmailServicePlugin()` bare. `objectstack serve` (and
the handle's own served-provider loop) constructs it with
`resolveCapabilityArgument('email', …)` from `@objectstack/core`, which reads
the deployment's mail configuration and, with none, picks the log transport
and a default no-reply sender. Built bare, the service had no sender: it
refused every notification email with `VALIDATION_FAILED: from address
required`, recorded each refusal as a failed sys_email row, and the outbox
retried each delivery about 1 s, 10 s and 60 s later, a stream of stray
inserts any later test's window could catch (#2175).
The harness now constructs it through that same rule. Measured on the
send_email file: every contact_welcome email delivery is `success` on its
first attempt and every sys_email row is `sent`, where on main each one was
refused and retried.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BDgipbNowaGa139em8Fhoq
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDgipbNowaGa139em8Fhoq
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
席位核对REWORK (one round) · PR #2177 at What is already right, read on the diff:
Two changes, then the PR is ACCEPT-ready:
The PR body names both changes and the evidence, per the review checklist's adjacency rule. No other change. Out of scope, one line each:
|
…nvironment, and the retry note is dated to its measurement
`resolveCapabilityArgument` falls back to `process.env` when it is handed no
`env`, so a shell carrying a real mail setup (`OS_EMAIL_PROVIDER=smtp`,
`OS_EMAIL_SMTP_*`, `OS_EMAIL_API_KEY`, `OS_EMAIL_FROM`) made the test boot
deliver through that transport. Measured: with `OS_EMAIL_PROVIDER=smtp` and an
SMTP host that refuses, the boot logged "using 'smtp' provider" and the
send_email pin went red on `expected [ 'queued', 'sent' ] to include
'failed'`. The harness now hands the rule `env: {}`, modelling an
unconfigured install whatever the machine's `OS_EMAIL_*`.
The `settleNotificationDeliveries` doc comment said, in the present tense,
that this boot's email channel fails every attempt for want of a from
address. Since the email service is constructed with a sender, it does not:
the sentence is now dated to the #2157 measurement, and keeps the general
point that a retry due within DELIVERY_NEAR_TERM_MS is waited out. Comment
only; the helper's code is unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BDgipbNowaGa139em8Fhoq
席位核对ACCEPT (#2175, maintainer direct dispatch 「同意你的建议」, CI health) · PR #2177 at
|
Fixes #2175
Clause-②: no
The
send_emailpin intest/script-bodies.test.tsnow reads thesys_emailinsert the engine answered with the idsend_emailreturned, not the first insert its recorder saw. The test boot'sEmailServicePluginis now constructed with the argumentobjectstack servegives it, from an empty mail environment, so a notification email on the test stack is delivered once instead of refused and retried, whatever mail settings the machine's shell carries inOS_EMAIL_*.Where the
failedinsert came from (measured on3d96e23, before any edit)The instrumentation ran in the worktree only and is not in this diff. It logged every
sys_emailinsert in the pin's window (the document, the engine's answer, the writer's stack) and dumped everysys_emailandsys_notification_deliveryrow when the window closed.contact_welcomenotification email, subjectNew contact: Ada Lovelace(or Prim Ary, Cam Paign, Twi Ce: one per contact a test in this file creates), addressed to the contact's owner,status: 'failed',error: 'rejected before delivery: VALIDATION_FAILED: from address required (set options.defaultFrom or pass input.from)'.NotificationDispatcher.runTick→processRow(@objectstack/service-messaging) → the email channel'ssend→EmailService.sendTemplate→send→sendInternal, wherenormalizeMessagerefuses the missing sender →EmailService.recordRejectedMessage(@objectstack/plugin-email), which inserts the failed row through the engine. It is notsend_email.EmailServicePlugin()has no sender, so every notification email is refused, and the outbox retries each delivery about 1 s and then about 10 s later. Each attempt writes one more failed row. In a quiet whole-file run, three of those 1 s retries were due within about 0.4 s of the pin's 52 ms window opening: a near miss. The dispatcher's loop, not the test, decides where they land.sys_emailinsert landed, then ran the action:AssertionError: expected 'failed' to be 'queued', the CI failure shape.[0]was theNew contact: Ada Lovelaceretry,[1]the body's ownHelloinsert.taskset -c 0beside two busy loops on that core,--maxWorkers=1, this file alone, through the shared lock): 6 of 6 runs passed on3d96e23. In the 5 runs with the row dump, the nearest pending email retry was 4.7 to 7.4 s after the window closed: the 1 s wave had already passed. The CI hit is timing-dependent; the staged run is the deterministic reproduction.PM hypotheses: (1) holds. The stray is the messaging dispatcher's notification email, the aftermath of an earlier contact insert (the
contact_welcomerecord-change flow) and its retry wave. (2) holds. The body hands its own row over withstatus: 'queued'written literally; in every probed run that insert wasqueuedand its stored rowsent, so matching it by the engine's answer is enough. (3) holds. The pin's window was 52 ms quiet and 216 to 834 ms under load, and no stray slowed it, sosettleNotificationDeliveriesis not used.The change
test/script-bodies.test.ts) awaits each recordedsys_emailinsert's answer (settled) and reads the one the engine answered withresult.emailId, asserting there is exactly one. Its other claims are unchanged: thequeuedhand-over, the['queued', 'sent']stored status, the activity pointer (source_object,source_idequal to the email's id,record_label), andresultequal to{ emailId, activityId }. The pointer comment now says what it proves for the row it picks: the pointer is the id the engine answered the body'ssys_emailinsert with.test/helpers/verify-stack.ts, theEmailServicePluginconstruction) now builds it withresolveCapabilityArgument('email', { stack: artifact, packageRoot: process.cwd(), providerModule: { resolveEmailCapabilityArg }, env: {} }).argument. The ruling's condition holds, read in the installed 18.0.0-next.3 packages:objectstack servebuilds each capability provider withresolveCapabilityArgument(cap, { stack: config, packageRoot, providerModule: mod, env: process.env }), thennew Ctor(arg)(@objectstack/clidist/commands/serve.js, lines 4506 to 4523).email, that isread(stack.email ?? {}, env, stack.appName).optionswithread=resolveEmailCapabilityArg(@objectstack/coredist/index.js, lines 2846 to 2848).resolveEmailCapabilityArgalways setsdefaultFrom. With noconfig.email.defaultFromand noOS_EMAIL_FROMit usesno-reply@plus the app-name slug plus.local(@objectstack/plugin-emaildist/index.mjs, lines 3227 to 3230). For this artifact it resolves to providerlog, senderno-reply@objectstack.localnamedObjectStack, and template contextappName: 'ObjectStack'.@objectstack/verify's own served-provider loop builds it by the same rule (dist/index.js, lines 392 to 410). The harness's bare instance pre-empted that.env: {}, the seat's review on this PR).CapabilityArgumentInputtakesenvon the installed dist (@objectstack/coredist/index.d.ts, lines 1368 to 1369: "The environment the mail and SMS readers read. Defaults toprocess.env."), and the rule falls back toprocess.envwhen it is handed none (dist/index.js, line 2845). Without the pin, a shell carrying a real mail setup made the suite deliver through it. The construction comment says so: the argument models an unconfigured install, whatever the machine'sOS_EMAIL_*.settleNotificationDeliveriesdoc comment, comment only (the seat's answer A on this PR's open question). Its present-tense sentence "This boot's email channel fails every attempt (its email service has no from address) …" became false with this PR. It is now dated to the test/flow-scheduled.test.ts: "case_sla_monitor … is a clean no-op when nothing has breached" times out at 5000 ms in CI (twice in one day, on two PRs); its time goes to work outside its own sweep #2157 measurement, before the email service was constructed with a sender. It keeps the general point: the outbox retries a delivery whose channel failed, and a retry due withinDELIVERY_NEAR_TERM_MSis waited out. The helper's code is unchanged.sleep, no retry wrapper, noskip, no raised timeout, no widened assertion.Before and after
3d96e23, staged overlapexpected 'failed' to be 'queued'; in the window:New contact: Ada Lovelacefailed, thenHelloqueued38d9093, harness still bare), same staged overlap3d96e23, natural loadcontact_welcomeemail deliverysuccesson its first attempt, everysys_emailrowsent: no failed rows, no retries1ff5add(noenvpin), shellOS_EMAIL_PROVIDER=smtp OS_EMAIL_SMTP_HOST=127.0.0.1 OS_EMAIL_SMTP_PORT=9(a refusing SMTP host), the pinusing 'smtp' provider;expected [ 'queued', 'sent' ] to include 'failed'107f07f(withenv: {}), the same shell, the pinOS_EMAIL_*107f07f(final head), natural load, whole fileWith the harness construction, the staged probe found no other writer's
sys_emailinsert within its wait: with no retry wave there was nothing left to land.Verification (
pnpm verifyon107f07f, the final head, through the shared lock)pnpm validate && pnpm typecheck && pnpm lint && pnpm lint:i18n-gate && pnpm hygiene && pnpm hygiene:tokens && pnpm build) printedVERDICT command-exit 0.tsccoverstest/**.pnpm exec vitest run --shard=1/3: 66 files, 1107 passed,VERDICT command-exit 0.--shard=2/3: 65 files, 1226 passed and 1 skipped,VERDICT command-exit 0.--shard=3/3: 65 files, 1579 passed,VERDICT command-exit 0.No test in
test/asserts on the failing-email shape (afailedsys_emailrow, an email retry). A grep forsys_email,'failed', delivery status and retry reads found none (the'failed'hits are automation-run statuses), and the whole suite stays green with the harness change. CI runs the rest.1ff5addmergesmainat2961679(PR #2173) into the branch;107f07fis the review round on top of it. The probe and staged runs were on3d96e23and38d9093.Acceptance notes
None of these is filed, and none is changed here.
env: {}pin covers the capability argument, not the mail settings namespace. The settings service readsOS_MAIL_*(themailnamespace's env keys,OS_plus namespace plus key) fromprocess.envwhen it is given noenv(@objectstack/service-settingsdist/index.js, line 573),@objectstack/verify'sbootStackconstructs it with none, and the email plugin rebuilds its transport from that namespace atkernel:ready. Measured on107f07f: a shell withOS_MAIL_PROVIDER=smtp OS_MAIL_SMTP_HOST=127.0.0.1 OS_MAIL_SMTP_PORT=9logsSMTP transport built from settings (host=127.0.0.1:9 …)and the pin goes red onexpected [ 'queued', 'sent' ] to include 'failed'. This predates this PR (the bare plugin read the same namespace), the settings plugin is the handle's and not the harness's, and CI sets noOS_MAIL_*. Carrier: none.CAPABILITY_PROVIDERS, "which is not exported". On 18.0.0-next.3 it is exported from@objectstack/core, and@objectstack/verify'sbootStackitself constructs every served provider when the caller hands no instance (constructServedProviders), including the always-onemailandmessaging. So part of theextraPluginslist may now duplicate the handle's own slate. This was already false before this PR, which does not touch it. Observation only; it was not measured as a defect. Carrier: none.VALIDATION_FAILED: from address required, a configuration error no retry can cure, as retryable. No public-door reach was measured, and a real install always derives a sender. Carrier: none.Generated by Claude Code