fix(core): plugin startup elapsed time is durationMs, the unit-bearing name its spec contract declares - #16057
Conversation
…ec contract already uses
`PluginStartupResult.startTime` has always carried `Date.now() - startTime`, an
elapsed duration, so the name asserts the opposite of the value: a reader who
correctly takes it for an instant and writes `Date.now() - result.startTime`
gets an age near the epoch. `packages/spec/src/kernel/startup-orchestrator.zod.ts`
already declares the correct name for the same measure (`duration`, "Time taken
to start the plugin in milliseconds"), and `PluginLoadResult.loadTime` twelve
lines above the defect already spells the identical computation truthfully -- so
this is a declared-vs-enforced divergence between `packages/core` and the spec
contract it implements, not a naming preference.
Three sites, all additive (nothing is removed, so no consumer changes):
- `PluginStartupResult` gains `duration?: number`; `startTime` stays,
populated with the same value, marked `@deprecated` with a doc comment that
states plainly what it holds (ADR-0087 L1 -- the old shape keeps working).
- the private `pluginStartTimes` map is renamed `pluginStartupDurations`
(private; measured zero readers outside `kernel.ts`).
- `getPluginStartupDurations()` is added and `getPluginMetrics()` becomes a
deprecated delegating alias.
Pin tests assert the value is a bounded elapsed duration rather than an
epoch-millisecond instant, on the success and the failure path -- the assertion
`toBeGreaterThan(0)` could never make.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
`PluginMetadata` requires `init`, so the object-literal `as` casts tripped TS2352 under `tsconfig.test.json`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…ctstack/core Additive widening of a published package's public surface (a new exported member on `PluginStartupResult`, a new method on `ObjectKernel`) takes at least `minor` per the `Check Changeset` step's WHICH LEVEL prose; the act wins over the `fix(` commit type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
📓 Docs Drift CheckThis PR changes 1 package(s): 10 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8f61445a7a078f26df3d8ce2cf9b39c6421358c1 && git checkout 8f61445a7a078f26df3d8ce2cf9b39c6421358c1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f377394ae2c2510636cdb12a34a49b5a442457d6 4458bcd49fbb6c27d2e8ec8e8f64fc4c8471b9ee && git checkout -B drift-repro f377394ae2c2510636cdb12a34a49b5a442457d6 && git merge --no-ff 4458bcd49fbb6c27d2e8ec8e8f64fc4c8471b9ee
node scripts/docs-audit/affected-docs.mjs --json f377394ae2c2510636cdb12a34a49b5a442457d6
|
…esult implements' Measured: `packages/core` neither imports nor references `packages/spec/src/kernel/startup-orchestrator.zod.ts`, and nothing in the repo implements `IStartupOrchestrator`. The two `PluginStartupResult` declarations describe the same domain result and share no shape, so 'implements' overstated a relationship that does not exist in code. The reason to take the contract's name is unchanged: it is the name the spec surface declares for this measure. Filed separately as the wider question this made visible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Contract review (clause ②) — NOT PASSED — one rename owed — PR #16057 at head
|
| # | claim | reading | verdict |
|---|---|---|---|
| 1 | PluginStartupResult.startTime holds Date.now() - startTime on both paths |
Diff context: const duration = Date.now() - startTime; … startTime: duration on the success and the failure return. Triage (5550872117) reproduced line for line. |
correct |
| 2 | The spec contract for the same measure declares duration |
origin/main packages/spec/src/kernel/startup-orchestrator.zod.ts:149 reads duration: z.number().min(0).describe('Time taken to start the plugin in milliseconds'). True on main today, false on the tree this repo has already ruled into being: PR #15906 (stack 3/6, tier-PASSed, queued to land into the stack branch) renames it to durationMs and tombstones duration (retired-keys/18.kernel__PluginStartupResult__duration.ts; startup-orchestrator.mdx: "duration never [REMOVED] … renamed to durationMs in @objectstack/spec 17"). The same PR renames StartupOptions.timeout → timeoutMs and totalDuration → totalDurationMs. |
stale premise |
| 3 | "durationMs cannot be the answer from a convention that is not yet in the repo" |
The gate is in the repo: packages/spec/scripts/check-duration-unit-keys.ts (759 lines) is on the stack branch since 2/6 landed at 22:40Z, seven minutes after this head. The convention is not a preference to wait on — it is two maintainer rulings: ruling B (5518649320: a duration-shaped number key carries its unit in its name, no baseline) and the population ruling B (5548763981: the rule governs every authored duration and every runtime-emitted duration). A per-plugin startup elapsed time on a public result type is a runtime-emitted duration. That the gate cannot see a TS interface in packages/core (it scans zod schemas; #15682 widens it to workspace zod schemas, not to interfaces) does not exempt the key from the rule — the maintainer's standing rule is 「开发应该对齐协议」, and the protocol this key aligns to spells it durationMs. |
incorrect |
| 4 | Additive widening: duration?: number added, startTime kept @deprecated; getPluginStartupDurations() added, getPluginMetrics() deprecated alias; private map renamed |
Diff read; dist/index.d.ts ablation as declared (37 lines, both legs proven to reach dist). Limb 1 fires, limb 2 does not. Clause-②: yes correctly declared and hung on PR and card. ADR-0087 L1 (old spellings kept, populated) is the right route for a public type member. |
correct, modulo the key name |
| 5 | Pins assert meaning (ceiling INSTANT_FLOOR_MS = 1e9), mutation proven |
Two tests; mutation to the instant fails both with the epoch-magnitude assertion. Sound. | correct |
| 6 | Changeset @objectstack/core minor, no ADR-0087 entry (nothing retired) |
Correct level for an additive public widening; the group is fixed so the level is moot in effect. No entry owed while nothing is removed. | correct |
| 7 | skills/objectstack-platform/SKILL.md deliberately untouched (governed; still true) |
Accepted — a code PR must not ride on skills/**. The page will teach a deprecated alias until the removal card; that is the removal card's docs edit. |
accepted |
The one change owed
Spell the new member durationMs?: number — the key PluginStartupResult in @objectstack/spec will carry once #15906 lands, and the spelling both rulings require for a runtime-emitted duration. Consequential edits, same round: the three doc comments and the changeset that cite PluginStartupResultSchema.duration cite durationMs; the pin tests read .durationMs; startTime: duration stays as the deprecated alias exactly as written. The method name getPluginStartupDurations() is not a key and may stay; the local const duration is not surface. Landing order is unconstrained — packages/core does not import the spec schema, so this can land before or after the stack.
Not asked: no change to the deprecation route, tests, or changeset level; no packages/spec edit here (#15906 owns that key).
Boundary flags
- PR body's "the spec key moves first and core follows in the same change" — the spec key is already moving in feat(spec)!: the fourteen kernel/ duration keys carry their unit in the key name (#15678, stack 3/6) #15906; the follow is this round, not a later one.
- Card core:
PluginStartupResult.startTimecarries an elapsed duration, and the spec contract for the same result declares it asduration#15820's suggested shape already allowed for this: "duration(or a unit-bearing name, if the convention landing in spec: duration-shaped number keys carry their unit in describe prose only — twottlkeys with different units in one block, baretimeoutkeys, unit-less tenant timeouts #14478 settles on one)". It has settled. - spec: the kernel startup-orchestrator contract (IStartupOrchestrator, PluginStartupResultSchema) is declared, exported and documented, and implemented by nothing #16059 (startup-orchestrator contract has zero implementers) is correctly filed, not folded. Noted for triage.
Evidence and gate
Checks on 4d20aa70: 33 success / 3 skipped / 0 red. check-governed-merges --test not run — no landing this round. needs:contract-review stays on this PR and on #15820. Patch round routed to the dispatching seat session_01ARYe3yQTQCUFm5qPYNgKaJ: push the rename on this branch (⛔ no rebase), post the new head here; this seat re-reads the delta and clears both carriers in one stroke.
Generated by Claude Code
…it-bearing name its spec contract declares Discharges contract review 5555409410 on PR #16057 (head 4d20aa7): the new published member was spelled `duration`, a key `packages/spec` has since retired. Measured on origin/main f377394: `packages/spec/src/kernel/startup-orchestrator.zod.ts:173` declares `durationMs`; `:176` tombstones `duration` with `retiredKey()` ("Rename the key to `durationMs`"); the gate `packages/spec/scripts/check-duration-unit-keys.ts` is in the tree (landed e9fcd6b). Two maintainer rulings make the rule govern every runtime-emitted duration, and a per-plugin startup elapsed time on a public result type is one. - `PluginStartupResult.durationMs?: number` replaces the never-released `duration?: number`; both emit sites in `kernel.ts` follow; `startTime` keeps its deprecated-alias treatment exactly as before. - The provenance sentence in the interface JSDoc (which ships in the published `dist/index.d.ts`), in `kernel.ts`'s `{@link}` and in the changeset cited the retired `PluginStartupResultSchema.duration`; all three now cite `durationMs` and say the bare spelling is retired. - The two pin tests read `.durationMs`; their ceiling assertion is unchanged. No ADR-0087 treatment on the core side: the spec tombstone entry (`packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginStartupResult__duration.ts`) records that core's interface is a different type and not a reader of the schema; core simply does not adopt the retired spelling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
|
Contract-review patch round — verdict 5555409410 (bound to head What changed, one commit (
Re-derived on The gate, run on a scratch merge of this head with Stated with it: that gate's population is numeric Zod chains — Clause-② re-measured at Local readings: core Generated by Claude Code |
duration, the name its spec contract already declaresdurationMs, the unit-bearing name its spec contract declares
Contract review (clause ②) — PASS, bound to head
|
Fixes #15820
PluginStartupResult.startTimehas always been assignedDate.now() - startTime, an elapsed duration, on both the success and the failure path. The name asserts the opposite of the value: a reader who correctly takesstartTimefor an instant and writesDate.now() - result.startTimegets an age near the epoch rather than a wait. An ambiguous name makes someone stop and check; this one lets them proceed confidently wrong.The root cause is a declared-versus-enforced divergence, not drift.
packages/spec/src/kernel/startup-orchestrator.zod.tsdeclares the same measure on the same result — the outcome of starting one plugin — asdurationMs: z.number().min(0).describe('Time taken to start the plugin in milliseconds')(:173onorigin/mainf377394ae) and tombstones the bare spelling at:176withretiredKey(...): "PluginStartupResult.durationwas renamed todurationMsin @objectstack/spec 17 — the unit of a duration-shaped number lives in the key name, not only in the describe prose. Rename the key todurationMs". So the contract surface was already correct andpackages/corehad drifted away from it. The same computation already has an honest name twelve lines above the defect in the same file:PluginLoadResult.loadTime(plugin-loader.ts:85, assigned at:182). This PR aligns core to the contract that exists; it invents no name and does not touchpackages/spec.Contract-review patch round — head
4458bcd49, discharging verdict 5555409410 on4d20aa70. The first head of this PR spelled the new memberduration, reasoning from theorigin/mainof 2026-09-05 (c463d03e), where the spec still declaredduration. That premise is stale: #15626 (e9fcd6bbd) landed the rename, the tombstone and the gatepackages/spec/scripts/check-duration-unit-keys.ts(packages/spec/package.jsoncheck:duration-unit-keys) onmain. Two maintainer rulings make the rule govern every runtime-emitted duration (5518649320, 5548763981), and a per-plugin startup elapsed time on a public result type is one. The patch commit renames the member todurationMs, follows at both emit sites and in the two pin tests, and corrects the provenance sentence — in the interface JSDoc that ships inside the publisheddist/index.d.ts, inkernel.ts's{@link}, in the changeset and in this body — which had cited the now-retiredPluginStartupResultSchema.duration. Nothing else in the round changed: thestartTimedeprecation route,getPluginStartupDurations()with thegetPluginMetrics()alias, and theminorchangeset are as reviewed. No ADR-0087 treatment is owed on the core side: the spec's own tombstone entry,packages/spec/src/migrations/entries/retired-keys/18.kernel__PluginStartupResult__duration.ts, records that core'sPluginStartupResultis a different type and not a reader of the schema; core simply does not adopt the retired spelling.One provenance claim in the card did not survive re-measurement, and the wording here is deliberate because of it. The card attributed the core-to-spec mapping to
packages/spec/api-surface/contracts.json:218and to thePluginStartupResultrows in the package CHANGELOGs. Measured:contracts.json:218ispackages/spec's own export listing, and the CHANGELOG row describes a consolidation insidepackages/spec— its own text says "Neither side had any consumer outside spec".packages/coreneither imports nor referencesstartup-orchestrator, and the twoPluginStartupResultdeclarations share a name and no shape. So there is no declared mapping row, and this PR does not claim one. The reason to take the contract's name is undamaged and is stated on its own terms:packages/specis this repo's one contract surface, it names this measuredurationMs, and core produced the same measure under a name that says the opposite. That is enough, and it does not depend on a mapping row.What changed — three sites, all additive
plugin-loader.tsinterface memberstartTime?: numberdurationMs?: numberadded;startTimekept, still populated, now@deprecatedwith a doc comment stating plainly that it holds elapsed milliseconds and not an instantkernel.tsprivate fieldpluginStartTimespluginStartupDurations(private; renamed outright)kernel.tspublic gettergetPluginMetrics()getPluginStartupDurations()added;getPluginMetrics()becomes a@deprecateddelegating alias returning the same mapNothing is removed, so no consumer has to change on this release. That is deliberate and is the ADR-0087 route for a public type member rather than deleting the key: L1 in the ADR's ladder — the old shape keeps working while the fleet moves — instead of an L3 break. Read
result.durationMswhere you readresult.startTime, andkernel.getPluginStartupDurations()where you calledkernel.getPluginMetrics(); the values are identical, so the move can be made at leisure.packages/core/ADVANCED_FEATURES.mdandpackages/core/examples/kernel-features-example.tsare updated so the in-package docs stop teaching the old name.skills/objectstack-platform/SKILL.mddocumentsgetPluginMetrics()and is deliberately not touched: the method still exists and still behaves identically, so that page is not wrong, andskills/**is a governed surface a code PR should not ride on.Clause-② — measured at
4458bcd49, both limbs answered separatelyMeasured with the
dist/index.d.tsablation on@objectstack/coreat4458bcd49: build at head,git restore --source=e1d4f9e3f(the merge base) on the two changed sources, rebuild, read the built declaration file, restore withgit checkout HEAD --on the two paths and prove it, rebuild. Mutation on disk before the base build: both source blobs equal to the merge-base blobs (81a869b5f/24c3c31c5),durationMscount 0 in both files. Restore: both blobs equal to the HEAD blobs (d541278af/a368dca10),git diff HEADempty, and the rebuilt headdist/index.d.tsbyte-identical to the pre-ablation one (cmp). Both builds proven by thedist/index.d.tsmtime moving (1788710748→1788711137→1788711148).Limb 1 — does any exported symbol or signature move? YES. Base-to-head, the published
dist/index.d.tsdiffers by 38 lines:PluginStartupResultgainsdurationMs?: number(head 1, base 0),ObjectKernelgainsgetPluginStartupDurations()returning aMapof plugin name to number (head 2, base 0), and the emitted private member changes name fromprivate pluginStartTimes;toprivate pluginStartupDurations;(a private member's existence is visible in the.d.ts; its signature is not). Read by exported-name set as well: base 260 names, head 260 names, added none, removed none (firing control:PluginStartupResultandObjectKernelboth in the set) — the widening is member-level and additive, no top-level export moves.duration?: numberappears 0 times in either build;.d.ctsagrees with.d.tson every count.Limb 2 — is any request newly accepted or rejected? NO. No schema, parser or validation path is touched and
packages/specis untouched. Every input the kernel accepted before it accepts now, and every rejection is unchanged. The only runtime difference is that two extra result members are populated with a value that was already computed.Limb 1 fires on its own, so this is
Clause-②: yes—needs:contract-reviewon this PR and on the card, and the PR stays draft until that review clears. The verdict recorded for4d20aa70does not cover this head; a re-review is owed on4458bcd49.The
#14478convergence question, re-answered on today'smainThe first head answered
durationbecause, at basee1d4f9e3f,check:duration-unit-keysdid not exist in the tree. Onorigin/mainf377394aeit does (packages/spec/package.json:290, landede9fcd6bbd), and the schema spells the keydurationMswithdurationretired. The card's own "or a unit-bearing name, if the convention landing in #14478 settles on one" has settled. Two readings of that gate, both stated because they answer different questions:src/**(its header; and--listmeasured on this tree: 0 rows frompackages/core/, 0 fromplugin-loader.ts, against 8 rows fromstartup-orchestrator.zod.ts). A TypeScript interface inpackages/coreis invisible to it, so its green is not evidence that this rename is right; the evidence is the diff and the built declaration above.f377394aein a scratch worktree, never pushed, 0 conflicts, my three core files byte-equal to the branch head and the spec schema byte-equal tomain:✓ check:duration-unit-keys — 215 duration-shaped numeric key(s) across 2326 source file(s) all carry their unit in the key name (or in a sibling unit, or under a declared exemption: 6 declared EpochMs instant(s), 11 declared externalVocabulary mirror(s)); zero offenders, no baseline.On this branch's own tree it exits 1 with 71 offenders — 70 inpackages/spec, 1 inpackages/drivers/driver-turso, 0 inpackages/core— the merge-base-era spellings of files this PR does not touch (0packages/specfiles in the diff), all already renamed onmain. Not a finding about this diff.The card's zero — re-run with a firing control, and one PM assumption came back FALSE
The card measured no readers of
getPluginMetrics()orpluginStartTimesoutsidepackages/core/src/kernel.ts, reverse-checked againstPluginStartupResult. Re-run whole-repo at base:pluginStartTimes— zero outsidekernel.tsholds: 3 hits, all inkernel.ts.getPluginMetrics— the zero does not hold. Seven hits outsidekernel.ts:packages/core/ADVANCED_FEATURES.md(2),packages/core/examples/kernel-features-example.ts(1),packages/core/src/kernel.test.ts(2), andskills/objectstack-platform/SKILL.md(2). The card's sweep was scoped topackages apps examples, which does not reachskills/.PluginStartupResultreturns 40-plus hits across the repo, so the search reaches.objectuiat its current checkout: zero hits for all three symbols, against a firing control (ObjectKernel/@objectstack/corehits across changesets, docs andpackages/app-shell).No promotion to p1. Every one of those readers treats the value as a duration already (
${duration}ms, "startup ms", "per-plugin startup durations",toBeGreaterThan(0)); none does instant arithmetic on it, and no out-of-repo consumer was measurable. Worth recording separately:PluginStartupResultis exported but no public method of@objectstack/corereturns one —startPluginWithTimeoutis private — so an external consumer would have to construct the shape itself to be misled by it.Tests
Two pin tests assert the meaning of the number, not merely that one is present.
toBeGreaterThan(0)cannot separate the two readings, because an epoch-millisecond instant passes it too; a ceiling can, since any instant today is around 1.7e12, orders of magnitude above any plugin'sstart(). The pins survive the rename unchanged apart from reading.durationMs; their mutation test at the first head (emit the start instant where the duration belongs) failed both withexpected 1788646580220 to be less than 1000000000, the epoch-instant reading the pin exists to catch, and the review re-ran it independently.Package suite and gates at
4458bcd49(the final commit), worktree at the branch head:pnpm --filter @objectstack/core test—Test Files 50 passed (50),Tests 1206 passed (1206).pnpm --filter @objectstack/core typecheck— exit 0;check:test-typecheck: OK — @objectstack/core's test layer compiles under packages/core/tsconfig.test.json; 4 file(s) / 4 error(s) / 4 pinned signature(s) held.pnpm lint— the full repo-wide run, exit 0, 0 problems. Not narrowed.pnpm check:nul-bytes—check-nul-bytes: OK (scanned 7739 text file(s) ... no raw ASCII control bytes); a control-byte self-scan of the four edited files: no hits.node scripts/check-adr-0087-registration.mjs --base e1d4f9e3f --head 4458bcd49(origin/main's copy of the script; the branch's differs) —this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).node scripts/check-changeset-no-major.mjs --base e1d4f9e3f --head 4458bcd49(origin/main's copy) —This diff introduces no major bump; its level axis printsNOT MEASUREDlocally (nopull_requestpayload to read a declaration from) — CI has the payload.node scripts/check-empty-changeset.mjs --base e1d4f9e3f—No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).And on the scratch merge tree described above, so that the gates read the scripts and the
packages/specCI's merge ref will use:pnpm --filter @objectstack/core test—Test Files 50 passed (50),Tests 1217 passed (1217)(mainadded eleven core tests since the merge base);typecheck—check:test-typecheck: OK.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackderives for the six pinned diff paths (git diff --name-only e1d4f9e3f 4458bcd49; origin/main's copy of the script atf377394ae): 52 exit 0; 2 exit 3 = PREREQUISITE NOT MET, not measured —check:dual-build-cjs-loads(76 packages unbuilt in the scratch tree; its own text: "This is NOT a pass: nothing was measured") andcheck:type-check-debt(--re-measurereports a stale tier itemisation on@objectstack/spec-monorepo, a maintainer's act to re-tally; its coverage half printedOK — 75/79 workspace packages type-checked).check:dts-closureswept the 4 built packages —core,metadata-core,spec,typesare the onlydist/directories in that tree —41/41 declared declaration file(s) present. Six further families the derivation marks NOT RUNNABLE LOCALLY ($RUNNER_TEMP, matrix shards) are CI's.Every exit code above was captured before any pipe (redirect-then-read), and each verdict is the gate's own printed line.
Changeset level
minor, and the act decides it rather than thefix(commit type: this is a purely additive widening of a published package's public surface (a new member on an exported interface, a new method onObjectKernel), which theCheck Changesetstep's WHICH LEVEL prose puts at least atminor. Nothing is removed, so there is no breaking declaration and no ADR-0087 ledger entry — the changeset states that disposition in writing anyway, and names the removal of the deprecated spellings as the change that will carry one.Out-of-scope finding, filed not fixed
Re-deriving the anchors surfaced a wider question that is not fixed here:
packages/spec's whole kernel startup-orchestrator contract —IStartupOrchestrator,PluginStartupResultSchema,StartupOptionsSchema,StartupOrchestrationResultSchema— is declared, exported and documented atcontent/docs/references/kernel/startup-orchestrator.mdx, and is implemented, parsed and consumed by nothing in the repo (zero implementers and zero schema consumers outsidepackages/spec, against a firing control). Filed as #16059 with the measurements, unassigned; it has since been ruled (card comment 5560225831) to land after this card so the two do not edit the same declaration concurrently. It is apackages/specquestion and folding it in here would have hidden it inside a rename.Authored by Claude Code in session
session_01ARYe3yQTQCUFm5qPYNgKaJ(https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ); the contract-review patch round ran in the same session.Generated by Claude Code