Skip to content

Commit b47ba2c

Browse files
os-warrenclaude
andauthored
fix(plugin-approvals): require warn on ApprovalServiceOptions logger sink (#10937)
Drops the `?` from `warn` on ApprovalServiceOptions['logger'] so a sink declaring an optional `error` always has a guaranteed durability-report channel (#9754, #10556). `error` stays optional. This is the thirteenth of the card's thirteen mechanical repairs -- held out of #10691 to serialize against PR #10547, which owned this file while open; that fence cleared when #10739 merged. Deletes the now-stale row from the shrink-only scripts/optional-error-sink-contract.baseline.json (the checker itself named the row as no-longer-red). The other two rows -- plugin-security's default-sink design call and service-settings' no-fallback design call -- are untouched; both remain escalated to the maintainer. No construction site needed a widening fix: the sole non-test call (ApprovalsServicePlugin.start) passes ctx.logger, whose warn is already required by the core Logger contract. Part of #10556. Claude-Session: https://claude.ai/code/session_0f14f70b-575c-5f2b-a235-4000a55db042 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9d101d2 commit b47ba2c

3 files changed

Lines changed: 46 additions & 12 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/plugin-approvals": minor
3+
---
4+
5+
**BREAKING** (compile-time only): `ApprovalServiceOptions['logger']` now
6+
declares a **non-optional** `warn`, so a durability report always has
7+
somewhere to land (#9754, #10556). This is the thirteenth of the thirteen
8+
mechanical repairs the card names — held out of #10691 to serialize against
9+
PR #10547, which owned `approval-service.ts` while it was open; that fence
10+
has since cleared.
11+
12+
`minor`, not `major`: during the launch window this stack ships breaking
13+
changes as `minor` — every publishable package versions in lockstep, so a
14+
`major` would promote the whole release. `patch` would be wrong in the other
15+
direction, because this *can* break a consumer's build. This is the same
16+
reasoning #10691 used for the twelve sibling repairs; no exemption for a
17+
types-only break was found there either, and none applies here.
18+
19+
`error` stays optional — hosts legitimately inject reduced sinks, and
20+
requiring `error` was measured and rejected as #9754 option C. What changes
21+
is that its *absence* now has a declared, guaranteed destination. Call sites
22+
keep the `logger?.warn?.(…)` spelling as the backstop for hosts the type
23+
cannot reach, so **no runtime behaviour changes**: nothing that printed
24+
before stops printing, and nothing silent starts printing.
25+
26+
### Who has to change, and what to do
27+
28+
Only a caller that constructs `ApprovalService` (or an `ApprovalServiceOptions`
29+
value) with a `logger` object that has **no `warn` method** — for example
30+
`{ error }` alone. Add a `warn` member; there is no rename, no removal, and no
31+
stored value or metadata key to rewrite. The only non-test construction site
32+
in this repo (`ApprovalsServicePlugin.start`, in this same package) passes the
33+
kernel `ctx.logger`, whose `warn` is already required, so the in-repo cost is
34+
zero.
35+
36+
<!-- adr-0087: not-required (runtime-interface-only packages/plugins/plugin-approvals/src/approval-service.ts#ApprovalServiceOptions) the tightened type is a plain TypeScript logger interface -- no Zod projection, no metadata surface, and it is referenced by none -- so `objectstack migrate meta` has nothing to rewrite. Nothing is removed or renamed and no stored value moves; the only consumer action is adding a `warn` member at a construction site the compiler names. -->

‎packages/plugins/plugin-approvals/src/approval-service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ function rowFromAction(row: any): ApprovalActionRow {
536536
export interface ApprovalServiceOptions {
537537
engine: ApprovalEngine;
538538
clock?: ApprovalClock;
539-
logger?: { info?: (msg: any, ...rest: any[]) => void; warn?: (msg: any, ...rest: any[]) => void; error?: (msg: any, ...rest: any[]) => void; debug?: (msg: any, ...rest: any[]) => void };
539+
logger?: { info?: (msg: any, ...rest: any[]) => void; warn: (msg: any, ...rest: any[]) => void; error?: (msg: any, ...rest: any[]) => void; debug?: (msg: any, ...rest: any[]) => void };
540540
/**
541541
* Optional automation surface used to resume a suspended flow run when a
542542
* decision finalises a request. Usually attached after construction via

‎scripts/optional-error-sink-contract.baseline.json‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,17 @@
2121
" metadata-protocol, plugin-approvals/lifecycle-hooks, both plugin-audit sinks,",
2222
" both plugin-auth sinks, plugin-email/attachment-reclaim, plugin-reports,",
2323
" plugin-sharing, plugin-webhooks and service-knowledge.",
24-
" = 3 — what is left, and NONE of the three is left for lack of effort:",
25-
" one is a serialisation leftover whose reason has expired (approval-service.ts),",
26-
" and two are DESIGN calls escalated to the maintainer rather than answered by a",
27-
" dev to make this checker green (security-plugin.ts, settings-service.types.ts)."
24+
" = 3 — what was left after #10556's first PR, and NONE of the three was left for lack",
25+
" of effort: one was a serialisation leftover whose reason had expired",
26+
" (approval-service.ts), and two are DESIGN calls escalated to the maintainer",
27+
" rather than answered by a dev to make this checker green (security-plugin.ts,",
28+
" settings-service.types.ts).",
29+
" -1 — #10556's follow-up dispatch repaired the serialisation leftover once its",
30+
" fence (PR #10547/#10739) cleared: `logger@ApprovalServiceOptions` dropped the",
31+
" `?` from `warn`, mechanically identical to the twelve already paid down.",
32+
" = 2 — both remaining rows are the escalated DESIGN calls; neither is a `?` deletion."
2833
],
2934
"entries": [
30-
{
31-
"file": "packages/plugins/plugin-approvals/src/approval-service.ts",
32-
"sink": "logger@ApprovalServiceOptions",
33-
"verdict": "optional-fallback",
34-
"members": "{ info? warn? error? debug? }",
35-
"note": "Approval-service options bag. Mechanically identical to the twelve repaired in #10556 — drop the `?` from `warn`. Held out of that PR to serialize against PR #10546, which owned this file while it was open. #10546 has since MERGED (2026-08-21), so the reason this row is still here has expired: it is a one-line repair awaiting a dispatch, not a design call and not an exemption."
36-
},
3735
{
3836
"file": "packages/plugins/plugin-security/src/security-plugin.ts",
3937
"sink": "logger@SecurityPlugin",

0 commit comments

Comments
 (0)