You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
* fix(convention): durability-costing degradations log error, not warn (#4632)
#4420: the durable suspended-run store attached to a table that was never
created. Every write failed into a warn nobody read, every restart dropped
all in-flight approvals, and the process reported perfect health throughout.
#4460 raised that ONE site to error. This makes it the rule, because the
class is what recurs.
The rule (AGENTS.md, new "Degradation log levels" section) is a judgment
QUESTION rather than an adjective, so it can be applied while writing the
catch: after the degradation, does the system still look "normal" from the
outside while something it claims is persisted has not actually landed?
Yes -> error. An error here owes the CONSEQUENCE and the FIX in its first
line, once, per the #4460 reference text in service-automation's start().
Anchored in AGENTS.md rather than an ADR addendum: this is an authoring
judgment every agent applies at every catch, not an architectural decision
scoped to one subsystem, and it is broader than the plugin-ordering context
ADR-0116 covers. It cross-references its two existing cousins -- Prime
Directive #10 (declared != enforced) and "Absence must be loud".
Sites raised to error, each judged individually:
- objectql schema sync, per object and summary (the info line said
"Schema sync complete" over a pass that lost DDL)
- objectql reload-time schema sync (a Studio edit shows a field the
column for which was never created)
- ObjectQL.syncSchemas() -- an EMPTY catch; marketplace install and
template seeding write into tables this failure means do not exist
- service-automation suspended wait-timer re-arm, 4 paths
Left at warn deliberately: the batch->sequential schema-sync fallback (it
recovers) and "no job service registered" on the re-arm path (a declared
absence, not a broken promise). Over-applying this rule trains everyone to
skim error, which is what made the original #4420 warn unreadable.
Teeth: pnpm check:durability-log-level walks the AST for catch blocks
guarding a declared vocabulary of durability-critical operations and fails
when one degrades below error without rethrowing. Follows same-file helpers
so extracting a reporter cannot quietly defeat it; ships a 10-case
--self-test; exceptions are hand-written and shrink-only with no --fix flag.
Deliberately narrow -- a ratchet, not a proof; both limitations are stated
in the script header.
Findings in the two packages frozen this round are filed, not fixed:
#4728 (metadata database-loader silent DDL swallow, baselined) and
#4729 (metadata-protocol seed-loader: its own comment demands error).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
* fix(objectql): pass the Error in Logger.error's SECOND slot, not the context bag
CI caught what a filter-scoped typecheck could not:
src/engine.ts(5356,15): error TS2353: Object literal may only specify known
properties, and 'object' does not exist in type 'Error'.
`Logger` (packages/spec/src/contracts/logger.ts) is asymmetric on purpose:
warn (message: string, meta?: Record<string, any>): void
error(message: string, error?: Error, meta?: Record<string, any>): void
Every escalation in the previous commit was written as a mechanical
warn -> error swap, which silently slides the context bag from the `meta`
slot into the `Error` slot. Excess-property checking makes that a hard
error -- but ONLY where a declaration is emitted, so it surfaced in the
tsup DTS build and in the repo-wide TypeScript Type Check, while every
runtime test stayed green (a plain object in the Error slot logs fine).
Four call sites corrected to (message, Error, meta):
- engine.ts syncSchemas() -> e as Error, { object, tableName, driver }
- plugin.ts reload-time schema sync -> the caught error
- plugin.ts reportSyncFailure() -> the caught error, { object, tableName, driver }
- plugin.ts pass summary -> undefined (no single error owns a whole pass)
The tests now assert the ARGUMENT POSITIONS rather than a flattened `meta`,
so this exact slide fails a test run too instead of only a build:
expect(args[0]).toBeInstanceOf(Error)
expect(args[1]).toMatchObject({ object: 'invoice' })
Also fixed the overdue-resume test: AutomationEngine.resume() REPORTS
machine-state problems in its result rather than throwing, so an
unregistered flow never reached the catch under test. It now injects a
throwing resume stub, which is what that catch actually guards.
Verified: turbo dependency-closure build, objectql + service-automation
builds WITH DTS, objectql typecheck, 7 + 25 tests, gate self-test 10/10.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(convention): a best-effort degradation that costs DURABILITY logs `error`, not `warn` — and a gate that enforces it (#4632)
7
+
8
+
#4420: the durable suspended-run store attached to a table that was never
9
+
created. Every write failed into a `warn` nobody read, every restart dropped all
10
+
in-flight approvals, and the process reported perfect health the entire time —
11
+
the symptom surfaced a release after the cause. #4460 raised that **one** site to
12
+
`error`. This makes it the rule, because the *class* is what recurs.
13
+
14
+
**The rule** (AGENTS.md → "Degradation log levels") is a question, not an
15
+
adjective, so an agent can apply it while writing the `catch`:
16
+
17
+
> After the degradation, does the system still look "normal" from the outside,
18
+
> while something it claims is persisted has not actually landed?
19
+
> Yes → `error`. No → `warn`/`info` is right.
20
+
21
+
An `error` here owes two things in its first line: the **consequence** (what is
22
+
not durable, and that the system will keep looking healthy anyway) and the
23
+
**fix** (the composition change that restores durability, or the explicit opt-out
24
+
that makes the degradation deliberate). Say it once, not once per failed write.
25
+
26
+
**Sites raised to `error`** — each was reviewed individually; escalating a
27
+
functional degradation is the mirror-image failure and was deliberately avoided:
28
+
29
+
| Where | What was silently lost |
30
+
|:---|:---|
31
+
|`objectql` schema sync, per object | DDL never ran — the object stays registered, routed and rendered while its table/columns do not exist |
32
+
|`objectql` schema sync, summary |`info: Schema sync complete` printed over a pass with failures; now an `error` naming the count |
33
+
|`objectql` reload-time schema sync | a Studio edit adds a field, the UI shows it, the API accepts it, the column was never created |
34
+
|`ObjectQL.syncSchemas()`| an **empty**`catch` — marketplace install and template seeding wrote into tables this failure means do not exist, then reported success |
35
+
|`service-automation` wait-timer re-arm (4 paths) | runs stay persisted but nothing re-arms them: every approval paused before the restart hangs forever |
36
+
37
+
**Deliberately left at `warn`** — the rule cuts both ways, and over-applying it
38
+
trains everyone to skim `error`: the batch→sequential schema-sync fallback (it
39
+
*recovers*), and "no job service is registered" on the re-arm path (a declared
40
+
absence in a host that never composed auto-resume — nothing was promised and
41
+
then broken).
42
+
43
+
**It has teeth.** A convention that lives only in AGENTS.md is the same
44
+
"declared ≠ enforced" shape this repo keeps paying to fix, so
45
+
`pnpm check:durability-log-level` walks the AST for `catch` blocks guarding a
46
+
declared vocabulary of durability-critical operations and fails when one
47
+
degrades below `error` without rethrowing. It follows same-file helpers (so
48
+
extracting a reporter cannot quietly defeat it) and ships its own `--self-test`.
49
+
Deliberately narrow: it cannot *discover* a new durability seam, only stop known
50
+
ones from regressing — extend `DURABILITY_CRITICAL_CALLEES` in the same PR that
51
+
fixes a new one.
52
+
53
+
No API, schema or behaviour changes — only the level, and the text, of what
0 commit comments