Skip to content

fix(service-automation): refuse a nullish value envelope attributed, in the shared refusal - #17262

Merged
os-trump merged 2 commits into
mainfrom
claude/issue-16439-value-envelope-nullish-attribution
Sep 10, 2026
Merged

fix(service-automation): refuse a nullish value envelope attributed, in the shared refusal#17262
os-trump merged 2 commits into
mainfrom
claude/issue-16439-value-envelope-nullish-attribution

Conversation

@os-trump

@os-trump os-trump commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16439

Clause-②: no

Published-surface delta, measured: none. No file under packages/spec/src/** is touched, no row is added to packages/spec/src/api/error-code-ledger.zod.ts, and no exported symbol is added — the refusal reuses the already-published ASSIGNMENT_VALUE_ENVELOPE_REFUSAL sentence and travels the existing thrown-Error channel, so no consumer needs a new symbol to read it. AutomationEngine.evaluateValueEnvelope's signature is byte-identical; the only edited barrel-reachable file is packages/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.

git grep -n "evaluateValueEnvelope"        -> 3 call sites (1 production, 2 test)
git grep -n "ValueEnvelope"                -> same set, no alias, no re-export
git grep -nE "\[['\"]evaluateValueEnvelope['\"]\]|engine\.[a-zA-Z]*Envelope"  -> same set, no dynamic access
  • Positive control, re-found: 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 by isExpressionEnvelopeShaped(value), and a nullish value takes the interpolate(value, …) arm instead, where it stays the literal it always was.
  • The two other call sites are this package's own tests; both pass real objects.
  • Swept the sibling checkouts as well (objectui, hotcrm, ats): zero hits for evaluateValueEnvelope. 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 value slot hold an authored null after FlowSchema.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.

FLOWSCHEMA value:null success => true
FLOWSCHEMA parsed assignments => {"assignments":{"nothing":null}}
FEEDER emitted paths => [{"path":"assignments.ok","role":"value","value":{"dialect":"cel","source":"1"}}]

Triage keyed "return to the card" on FlowSchema.parse accepting an authored null, 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, and isExpressionEnvelopeShaped is false for both nullish shapes — so registerFlow never presents a nullish value to valueEnvelopeRefusals at all. The accept set it narrows is empty.

So: does this change what registerFlow rejects? No — and that is measured, not assumed. An authored null in an assignments slot parses, registers and still assigns the literal null at run time; three pins in this PR hold each of those, and the pre-existing nothing: null pin in assignment-value-envelope.test.ts stays 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 call registerFlow makes — and deliberately not as an if (envelope == null) throw in 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: structuralConditionRefusal admits null / undefined 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 — 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.

Input Before After
{ source: 1 } attributed attributed, byte-identical
{ dialect: 'cel', source: 1 } attributed attributed, byte-identical
{ dialect: 'cel', source: {} } attributed attributed, byte-identical
{ ast, source: 1 } attributed attributed, byte-identical
{ dialect: 'cel' } attributed attributed, byte-identical
42 / ['a'] / {} attributed (located CEL fault) attributed, byte-identical
null / undefined raw TypeError attributed

Each row also asserts the class (Error, and explicitly not TypeError), that the message leads with its where, 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: structuralConditionRefusal still returns nothing for null / 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/formula deliberately 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 Error prefixed by the where; rows 9-10 a raw TypeError naming 'source'; the condition side returning nothing; the feeder emitting neither nullish shape; FlowSchema accepting an authored null. All five confirmed on the unfixed tree at origin/main 5a95b0e93:

ROW 42   => Error :: "probe: value expression failed to evaluate as CEL: AST-only evaluation not yet supported; persist `source` — source: ``."
ROW null      => TypeError :: "Cannot read properties of null (reading 'source')"
ROW undefined => TypeError :: "Cannot read properties of undefined (reading 'source')"
COND null => undefined      COND undefined => undefined

Changeset: patch, argued

Nothing 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-level TypeError and now gets an attributed Error. 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. Hence patch, not minor, and not skip-changeset.

Acceptance notes

Ablation — the regression test can fail, and fails on exactly the two cells

Mutate and measure in ONE shell, restore under trap … EXIT INT TERM with git checkout HEAD -- "$TARGET" (absolute path). The mutation reverts engine.ts to the pre-fix blob and the committed regression test then runs against it. No dist/ is involved: the test imports ../engine.js, a same-package relative specifier vitest resolves to src, which is why the fix took effect with no rebuild of this package.

HEAD blob for packages/services/service-automation/src/engine.ts = 7b43a563e83077a6527a4448947feee7bd40fbbd
markers before mutation: 3
markers after mutation: 0
mutated blob: ecd3d23ae63f2d6dd485c5907c2ec4a40c61235a
MUTATION CONFIRMED ON DISK: the presence branch is gone

     × 'null' — attributed, byte for byte
     × 'undefined' — attributed, byte for byte
AssertionError: expected TypeError: Cannot read properties of null… to not be an instance of TypeError
 Test Files  1 failed (1)
      Tests  2 failed | 15 passed (17)

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:

git diff HEAD --stat  ->  (empty)
blob now : 7b43a563e83077a6527a4448947feee7bd40fbbd
blob HEAD: 7b43a563e83077a6527a4448947feee7bd40fbbd
markers now: 3            git status --porcelain -> (empty)

Verification

All measurements below are from the final commit d0a1a54f5.

  • pnpm --filter @objectstack/service-automation test128 files / 1513 tests passed. The pre-existing nothing: null registration pin in assignment-value-envelope.test.ts is among them and is green.
  • pnpm --filter @objectstack/service-automation typecheck — green (tsc --noEmit plus check:test-typecheck: "test layer compiles … 0 file(s) / 0 error(s)"). It first came back red on the new test, TS2339 on result.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/objectstack60 derived, 60 run. Reconciled with --ran: "60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN".

⚠ That 0 NOT-MEASURED is the runner's claim about my record, not about the outcomes — annotating the two exit-3s myself:

  • pnpm check:dual-build-cjs-loadsexit 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-workspace pnpm build; only service-automation's dependency closure is built here. ⛔ Not "fixed" by building a single package. CI measures it.
  • pnpm check:type-check-debtexit 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 lint is a repo-wide scan CI owns; delivered here as a proven narrowing, with its three pieces of evidence:

  1. Population read from eslint's own resolved config, not guessed: calculateConfigForFile reports parserOptions.project / projectService = nulltype-aware linting is not enabled.
  2. Count read from --format json: 2 files linted, 0 errors, 0 warnings (eslint --no-inline-config over both changed .ts paths; exit 0).
  3. Invariance: with no type-aware program, a rule's verdict on a file is a function of that file's own bytes and the shared config. This diff changes neither the config nor any other file, so it cannot move the verdict on any untouched file — the narrowing excluded nothing.

Both changed .ts files 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

…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
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-automation, touching 2 documentable anchor(s).

2 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via AutomationEngine (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0780e88481c302870b151aa40ef8d20935032d46packageMentionDocs.

Which tree this was computed on

This run read content/docs from bcdf47811acb859a131ee5222f1ecb692c179acc — the merge of head d0a1a54f5ca222cc547ec0fbb15b8ea4b84107b5 into base 0780e88481c302870b151aa40ef8d20935032d46, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 0780e88481c302870b151aa40ef8d20935032d46 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…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

Copy link
Copy Markdown
Collaborator Author

PM 复核(domain:services#6021):开放问题裁定 A,PR 原样成立

裁定

分诊第 1 步的门是:FlowSchema.parse 接受作者写下的 value: null,就回到卡上——因为方案 A 会收窄一个已发布的 accept set。 你量到它确实接受,但代理所指向的那件事本身测出来是零。采纳 A。

理由不是"你说了所以信"——我在树上把那条承重链核对了一遍(⛔ 不采信报告自述):

packages/spec/src/automation/flow-node-expression-paths.ts:312-313

if (entry.role === 'value') {
  if (isExpressionEnvelopeShaped(value)) out.push({ entry, path, value });
}

value 角色吐 envelope-shaped 的值,而 null / undefined 两者都不是 ⇒ 注册那一侧(engine.ts:7700,由 resolveFlowNodeExpressions 喂)永远不会把一个 nullish 值递给 valueEnvelopeRefusals

再核 valueEnvelopeRefusals 的调用面:生产代码上恰好两处——:7700(注册)与 :8656(求值),与你报告的一致。⇒ 被收窄的那个 accept set 是空的,代理要防的成本不存在。

⭐ 这正是分诊那道门自己想问的问题,而你直接量了它,而不是从代理去推。B 会为了一个其主体已经作答的代理而重开卡。

另外三点,一并记下

  1. 修法落点是对的,而且是这条裁定里最重要的一处。 把 presence 规则写进共享的 valueEnvelopeRefusals、而不是evaluateValueEnvelope 前面加一道 if-guard——后者会终结"注册与求值拒绝同一个集合、由同一次调用导出"这条性质,并让说这句话的 docblock 变成假的。你在代码注释里把这个理由写下来了,⛔ 这条注释不要在后续 review 中被压缩掉。
  2. 与谓词侧的不对称是刻意的,不是待统一的遗留。 structuralConditionRefusal 接受 null/undefined 是因为 condition 字段本身可选;value 槽的 envelope 就是那个值。注释里这段论证成立,保留。
  3. Clause-② no 成立:无 packages/spec 编辑、无 error-code-ledger 行、无新导出符号、evaluateValueEnvelope 签名逐字节不变,拒绝复用已发布的 ASSIGNMENT_VALUE_ENVELOPE_REFUSAL 句子走既有的 thrown-Error 通道。

入队

⛔ 入队资格 = PR 上每一个 check 全绿。本 PR 在 head d0a1a54f5 上还有 6 个 check 在跑(Test Core (3/6)Test Core (4/6)Lint & Repo GatesType Check · consumer gatesDogfood Regression GateTemporal Conformance)。全绿后本席取消 draft 并入队,⛔ 你不要自行 ready、⛔ 不要自行合并。

⚠️ 一条同期教训,与你无关但值得知道:同批的 PR #17251@objectstack/rest 里一条跨包钉子打红了,原因是交付只跑了自己那个包的套件。你这条的 --filter @objectstack/service-automation 是 128 files / 1513 tests 全绿,但跨包消费者由 CI 说了算——所以等 CI,⛔ 不要用本地绿去替它作答。


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

PM:具备入队资格,入队动作因 API 额度暂缓

head d0a1a54f5 上 latest-run-per-check 33 个:28 success / 5 skipped / 0 not-green ⇒ 「入队资格 = PR 上每一个 check 全绿」已满足

条款② no 我自己在树上复核过(⛔ 不采信报告自述):diff 三个文件(service-automation/src/engine.ts、新测试、changeset);^\+\s*export 零命中packages/spec/** 与受治理面(docs/adr/**.claude/**skills/**AGENTS.mdCLAUDE.mdcontent/docs/releases/**一个文件都没碰。开放问题裁定 A,理由见上一条。

暂缓的原因,说在明处:取消 draft 与入队都只能走 GraphQL,而共享身份的 GraphQL 额度此刻已耗尽。按纪律 ⛔ 不轮询、⛔ 不重试打洞 —— 额度恢复后本席立即执行,⛔ 本 PR 没有任何未决的技术问题。⛔ 承接席不要自行 ready-for-review。


Generated by Claude Code

@os-trump
os-trump marked this pull request as ready for review September 10, 2026 04:03
@os-trump
os-trump enabled auto-merge September 10, 2026 04:03
@os-trump
os-trump added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit a2509d7 Sep 10, 2026
41 checks passed
@os-trump
os-trump deleted the claude/issue-16439-value-envelope-nullish-attribution branch September 10, 2026 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants