fix(service-automation): refuse a nullish value envelope attributed, in the shared refusal - #17262
Conversation
…in the shared refusal (#16439) `AutomationEngine.evaluateValueEnvelope` derives its verdict from `valueEnvelopeRefusals` — the same call `registerFlow` makes — so registration's reject set and evaluation's reject set are one set by construction. That covered every malformed envelope and exactly two shapes fell outside it: `null` and `undefined`. Neither published primitive judges them (the shape rule is a no-op off `isExpressionEnvelopeShaped`; `validateExpression` reads an absent `source` as "not authored"), so both returned no findings and the method read `envelope.source` off nothing — a bare `TypeError` with no `where`, no source and no rule. The presence rule is stated in the SHARED refusal, not as a guard in the evaluator: a reject reason living only on the evaluation side would end the property this design exists for, and the docblock that says so would stop being true. Refused rather than admitted. The asymmetry with the predicate path is deliberate: `structuralConditionRefusal` admits nullish because the condition field is optional, so absence there means "the author wrote no predicate". A value slot's envelope IS the value, so an absent one is a caller handing nothing where a value was required. Measured, not assumed: the value-role feeder (`resolveFlowNodeExpressions`) emits only envelope-shaped objects, so `registerFlow` never presents a nullish value to the shared refusal — this adds nothing to the registration reject set. An authored `null` in an `assignments` slot still parses (`FlowSchema.parse` accepts it) and still registers. The regression is the whole table, not the two repaired cells: all ten shapes the card enumerates are pinned to their messages byte for byte, so the eight already attributed are shown to be untouched. The condition side is pinned too, unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 6 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 bcdf47811acb859a131ee5222f1ecb692c179acc && git checkout bcdf47811acb859a131ee5222f1ecb692c179acc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0780e88481c302870b151aa40ef8d20935032d46 d0a1a54f5ca222cc547ec0fbb15b8ea4b84107b5 && git checkout -B drift-repro 0780e88481c302870b151aa40ef8d20935032d46 && git merge --no-ff d0a1a54f5ca222cc547ec0fbb15b8ea4b84107b5
node scripts/docs-audit/affected-docs.mjs --json 0780e88481c302870b151aa40ef8d20935032d46
|
…elope pin (#16439) `ExecutionResult.output` is typed `{}`, so reading `.digest` off it was a TS2339 the package's own `typecheck` catches (its tsconfig reaches every `*.test.ts`, which is how this surfaced). Narrowed at the read; the assertion is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
PM 复核(
|
PM:具备入队资格,入队动作因 API 额度暂缓head 条款② ⛔ 暂缓的原因,说在明处:取消 draft 与入队都只能走 GraphQL,而共享身份的 GraphQL 额度此刻已耗尽。按纪律 ⛔ 不轮询、⛔ 不重试打洞 —— 额度恢复后本席立即执行,⛔ 本 PR 没有任何未决的技术问题。⛔ 承接席不要自行 ready-for-review。 Generated by Claude Code |
Fixes #16439
Clause-②: no
Published-surface delta, measured: none. No file under
packages/spec/src/**is touched, no row is added topackages/spec/src/api/error-code-ledger.zod.ts, and no exported symbol is added — the refusal reuses the already-publishedASSIGNMENT_VALUE_ENVELOPE_REFUSALsentence and travels the existing thrown-Errorchannel, so no consumer needs a new symbol to read it.AutomationEngine.evaluateValueEnvelope's signature is byte-identical; the only edited barrel-reachable file ispackages/services/service-automation/src/engine.ts, whose exported names are unchanged.The two measurements taken before a line was written
1. The overturning measurement — does any direct caller treat a nullish envelope as "no assignment"?
Zero, and the sweep can answer "yes": the positive control re-found the known production call site.
packages/services/service-automation/src/builtin/logic-nodes.ts:169— and it does not treat a nullish envelope as "no assignment", because it cannot deliver one: the call is guarded byisExpressionEnvelopeShaped(value), and a nullish value takes theinterpolate(value, …)arm instead, where it stays the literal it always was.objectui,hotcrm,ats): zero hits forevaluateValueEnvelope. Callers outside these repos remain, and stay, unknowable — stated as unknown, not as "none".⇒ nothing to overturn the ruling. REFUSE stands.
2. Triage's first-step reading — can a
valueslot hold an authorednullafterFlowSchema.parse?Yes, it can — and the branch that reading was standing proxy for still measures zero cost. Both readings are pinned in the test file so neither goes stale.
Triage keyed "return to the card" on
FlowSchema.parseaccepting an authorednull, on the reasoning that option A would then start refusing flows that register today. The third line falsifies that inference directly:registerFlow's value-role feeder,resolveFlowNodeExpressions, emits only envelope-shaped objects, andisExpressionEnvelopeShapedisfalsefor both nullish shapes — soregisterFlownever presents a nullish value tovalueEnvelopeRefusalsat all. The accept set it narrows is empty.So: does this change what
registerFlowrejects? No — and that is measured, not assumed. An authorednullin anassignmentsslot parses, registers and still assigns the literalnullat run time; three pins in this PR hold each of those, and the pre-existingnothing: nullpin inassignment-value-envelope.test.tsstays green. The proxy reading says "accepts"; the mechanism it stood for says "zero". Flagging the divergence rather than deciding it silently — if the seat wants the literal branch honoured over the mechanism it was a proxy for, say so and this can be re-opened.What landed
The presence rule is stated in the shared
valueEnvelopeRefusals— the same callregisterFlowmakes — and deliberately not as anif (envelope == null) throwin front of the evaluator. A reject reason living only on the evaluation side would end the property this design exists for, and the docblock that asserts it would stop being true. Both halves learn the shape at once; the registration half simply never gets handed one, which is a property of its feeder, not a second reject set.Refused rather than admitted. The asymmetry with the predicate path is deliberate and is now written down where it can be found:
structuralConditionRefusaladmitsnull/undefinedbecause the condition field is optional, so absence there means "the author wrote no predicate". A value slot's envelope is the value, so an absent one is a caller handing nothing where a value was required — nothing to compute, and no falsy default to hide behind.The regression is the whole table
All ten shapes from the card are pinned in one test, each to its message byte for byte — the eight already attributed today, so the fix is shown to have left them alone, and the two that now join them.
{ source: 1 }{ dialect: 'cel', source: 1 }{ dialect: 'cel', source: {} }{ ast, source: 1 }{ dialect: 'cel' }42/['a']/{}null/undefinedTypeErrorEach row also asserts the class (
Error, and explicitly notTypeError), that the message leads with itswhere, and that it carries a source — so a future refactor cannot satisfy the string pin with a throw that is still a language-level fault. The condition side is pinned unchanged in the same file:structuralConditionRefusalstill returns nothing fornull/undefined, and still refuses what it always refused.The eight literals are composed from the published constants wherever one exists (
ASSIGNMENT_VALUE_ENVELOPE_REFUSAL,EVALUATED_EXPRESSION_SOURCE_REQUIRED), so this file re-spells no sentence that has an owner. Two fragments are literal because@objectstack/formuladeliberately does not export them; the file says so, and says that a legitimate rewording there re-captures the table rather than loosening it.Red first
Prediction was written before the run: rows 1-8 an attributed
Errorprefixed by thewhere; rows 9-10 a rawTypeErrornaming'source'; the condition side returning nothing; the feeder emitting neither nullish shape;FlowSchemaaccepting an authorednull. All five confirmed on the unfixed tree atorigin/main5a95b0e93:Changeset:
patch, arguedNothing changes for authored metadata — the only production call site guards with
isExpressionEnvelopeShaped, and the feeder reading above shows registration cannot reach the new rule either. What does move is the runtime behaviour of a public method on an exported class: a direct caller that passed a nullish envelope used to get a language-levelTypeErrorand now gets an attributedError. That is a published surface, so it is not nothing — but it adds no API, no option and no capability, and no correct caller has to adapt. Hencepatch, notminor, and notskip-changeset.Acceptance notes
service-automation/src/engine.ts. service-automation: the two INITIAL-execution completion paths still callrecordLog({ status: 'completed' })inside the node-failuretry— a history-write throw reports a finished run asfailed#16274 and [finding] a durably-PAUSED child that fails after resume rolls NO totals to its parent — the parent's steps are frozen at suspend time, so #15617's own "a failing child is counted" control does not hold on that path #16666 are queued against the same package but are not open PRs; PR fix(approvals): classify every ExecutionStatus member for the dead-run sweep, so arefusedrun releases its pending approval #17248 is onplugin-approvals.?.in(value as { source?: unknown })?.source, immediately below the new presence branch, is now unreachable-as-nullish. Left as written: removing it changes no behaviour and would only widen the diff. noted, not filed.Ablation — the regression test can fail, and fails on exactly the two cells
Mutate and measure in ONE shell, restore under
trap … EXIT INT TERMwithgit checkout HEAD -- "$TARGET"(absolute path). The mutation revertsengine.tsto the pre-fix blob and the committed regression test then runs against it. Nodist/is involved: the test imports../engine.js, a same-package relative specifier vitest resolves tosrc, which is why the fix took effect with no rebuild of this package.Exactly the two repaired cells fail; the eight already-attributed rows and both condition-side pins pass unchanged — which is the table's whole point stated as a measurement rather than a claim.
Restoration proven by state, never by an exit code:
Verification
All measurements below are from the final commit
d0a1a54f5.pnpm --filter @objectstack/service-automation test— 128 files / 1513 tests passed. The pre-existingnothing: nullregistration pin inassignment-value-envelope.test.tsis among them and is green.pnpm --filter @objectstack/service-automation typecheck— green (tsc --noEmitpluscheck:test-typecheck: "test layer compiles … 0 file(s) / 0 error(s)"). It first came back red on the new test, TS2339 onresult.output.digest; that is the second commit here, and it is also the evidence that this package's tsconfig really does reach its*.test.ts.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack— 60 derived, 60 run. Reconciled with--ran: "60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN".⚠ That
0 NOT-MEASUREDis the runner's claim about my record, not about the outcomes — annotating the two exit-3s myself:pnpm check:dual-build-cjs-loads— exit 3, PREREQUISITE NOT MET, not a pass. "this gate reads built output, and some package has no dist/" (50 packages listed). It wants a whole-workspacepnpm build; onlyservice-automation's dependency closure is built here. ⛔ Not "fixed" by building a single package. CI measures it.pnpm check:type-check-debt— exit 3, PREREQUISITE NOT MET, not a pass. "12 workspace dependenc(ies) … have no built type entry point on disk"; the gate's own text says measuring from here "would silently measure a DIFFERENT WORLD" and that no ledger entry may be raised on it. CI measures it.⇒ honest tally: 60 derived / 60 run / 58 measured green / 2 NOT MEASURED (both exit-3 prerequisite, both whole-workspace-build gates, neither caused by this diff) / 0 UNRUN. Also re-run on the final head after the second commit:
check:nul-bytes,check:type-check-coverage,check:cross-package-test-inputs,check:test-source-alias,check:objectql-double-limit,check:where-matcher— all exit 0.pnpm lintis a repo-wide scan CI owns; delivered here as a proven narrowing, with its three pieces of evidence:calculateConfigForFilereportsparserOptions.project/projectService=null⇒ type-aware linting is not enabled.--format json: 2 files linted, 0 errors, 0 warnings (eslint --no-inline-configover both changed.tspaths; exit 0).Both changed
.tsfiles were also scanned for control characters beyond the gate (grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'): no hits.🤖 Generated with Claude Code
https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Generated by Claude Code