Skip to content

refactor(service-automation): deprecate non-canonical CRUD node config-key aliases - #2425

Merged
os-zhuang merged 1 commit into
mainfrom
claude/sad-yonath-1f92c5
Jun 28, 2026
Merged

refactor(service-automation): deprecate non-canonical CRUD node config-key aliases#2425
os-zhuang merged 1 commit into
mainfrom
claude/sad-yonath-1f92c5

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Audit: "lenient input" config-key aliases in automation node executors

FlowNodeSchema.config is z.record(z.string(), z.unknown()) (flow.zod.ts:102), so the spec blesses no particular config key — the executor is the only runtime gate on key names. That makes every cfg.canonical ?? cfg.alias in a node executor a Postel's-law tolerance that fossilizes a wrong shape into a de-facto second contract and hides metadata-generation bugs. This PR is Step 1 of converging them: it follows the fields/fieldValues precedent (reject the wrong shape at author/publish time; don't absorb it at runtime).

What this PR changes (clear-cut, zero-risk)

  • readAliasedConfig() helper — returns the canonical value; on the alias path, emits a one-time logger.warn naming the canonical key, then keeps working. (graph-lint runs only at publish, so it can't reach flows already stored in prod that are never re-published — the runtime warn covers that population.)
  • Wired into get/create/update/delete_record for objectName and filter.
  • Fixed the one authoring source that taught the alias: skills/objectstack-automation/SKILL.mdupdate_record example object:objectName: (every example .flow.ts already uses objectName/filter).

No alias is removed — all are plausibly present in stored prod flows. Removal is a tracked follow-up gated on (a) the warn window and (b) a cloud graph-lint rule.

Findings table (full audit)

Canonical is established by docs/skill/examples + cloud graph-lint (the spec is z.record, vacuously permissive). "Debt" = one canonical key + tolerated alias; "Union" = spec genuinely blesses both.

#Alias (file:line)Spec saysClassAction
1crud-nodes.tsobjectName ?? object (get/create/update/delete)z.record (any key)DebtobjectName canonical (all 8 example flows); object only in SKILL.md✅ this PR: warn + fix skill
2crud-nodes.tsfilter ?? filters (get/update/delete)z.recordDebtfilter canonical; filters zero usage✅ this PR: warn
3notify-node.ts:75-77recipients ?? to, title ?? subject, message ?? bodyz.recordDebt — email-flavored aliases; notify unused in examplesFollow-up: same warn treatment
4screen-nodes.ts:198inputs ?? input (script)z.recordDebt — singular slip; zero usageFollow-up: warn
5screen-nodes.ts:135function ?? functionName (script)z.recordDebt — deliberate (in-code: "AI/templates commonly emit it", #1870)Fix AI/skill emission first, then warn (platform self-emits it today)
6wait-node.ts:61,108… ?? loose.duration, … ?? loose.signalwaitEventConfig.timerDuration/signalName (typed)Debt — off-spec short aliases; zero usageFollow-up: remove/warn (low traffic)
7wait-node.tswaitEventConfig.* ?? config.*spec defines waitEventConfig; examples author under configStructural dual-home (borderline)Pick one home + document; lower priority
8logic-nodes.ts:71variable ?? name ?? key (assignment)z.record (+ documented 3-surface normalizer)Debt slice in a legit multi-shape normalizerKeep normalizer; warn on name/key array-item aliases
9action.zod.ts:529p.name ?? p.field (+ refine :70).refine(name || field) blesses bothLegitimate union✅ Leave as-is
10http-dispatcher.ts:2836ctxBody.objectName ?? ctxBody.object (API POST /:name/trigger)n/a (HTTP body)Debt — same as #1, on the trigger surfaceConverge with #1 (separate PR; runtime pkg)
11http-dispatcher.ts:2892inputs ?? variables (resume body)n/aDebt (lean) / low trafficFollow-up

Out of scope (separate convergence tracks — flagged, not folded in)

  • Query-DSL multi-spellingmetadata-protocol/protocol.ts:2060,2077 (orderBy ?? sort, filter ?? filters ?? $filter ?? where) and http-dispatcher.ts:1792 (where ?? filter ?? filters): mostly a legit OData/ObjectQL union; filters plural is the debt slice. Owner: ObjectQL query options.
  • REST camelCase↔snake_caserest-server.ts (~40 sites, e.g. body.object ?? body.object_name): a deliberate, documented REST body convention. Leave.
  • DB row column reconciliationplugin-approvals/approval-service.ts (raw.flow_node_id ?? raw.current_step), auth/security *_id ?? *Id: reading persisted rows, not author config. Separate (legacy column) concern.
  • Spec-ambiguousservice-datasource/default-datasource-driver-factory.ts:92user ?? username: datasource.zod prose says user, the postgres/mongo driver schemas say username. Fix the spec first (pick one), then converge — not clean runtime debt.

Why deprecation window, not straight removal

The spec is z.record, so the runtime has been the only gate; any tolerated alias may sit in a stored prod flow and there's no telemetry to prove otherwise. Several are documented/intuitive (object is literally in the skill; to/subject read as email; filters/input/duration are natural slips). Removal's failure mode is loud-and-safe (create_record: objectName required) rather than corruption — but it's still an availability regression for running flows, which is exactly what the window avoids. So: warn now, remove after the window + a graph-lint rule, and stop teaching/emitting the alias at the source.

Tests

  • @objectstack/service-automation: 225 pass (incl. new crud-config-aliases.test.ts — alias still resolves, warns once per alias, canonical path silent).
  • @objectstack/spec: 6600 pass.
  • tsup ESM/CJS/DTS build clean.

Suggested follow-ups

  1. Apply the same readAliasedConfig warn to notify/screen/wait/logic aliases (Implement ObjectStack protocol specification with Zod schemas and TypeScript interfaces #3,4,6,8) and the http-dispatcher trigger object (chore: version packages #10).
  2. Add the cloud graph-lint rule rejecting non-canonical CRUD config keys at publish.
  3. File the removal-tracking issue; remove alias paths after one release with the warn + graph-lint enforcing.
  4. Resolve the datasource user/usernamespec ambiguity before touching that runtime tolerance.

🤖 Generated with Claude Code

…g-key aliases
Flow node `config` is `z.record(z.string(), z.unknown())` in spec, so the
executor is the only runtime gate on key names. The CRUD nodes quietly accepted
`object` for `objectName` and `filters` for `filter` via `cfg.canonical ?? cfg.alias`
— a Postel's-law tolerance that fossilizes the wrong shape into a de-facto second
contract and hides metadata-generation bugs.
Converge on the canonical key (matching the `fields`/`fieldValues` decision: reject
the wrong shape at author/publish time, don't absorb it at runtime). This is the
deprecation-window step, not a removal:
- add `readAliasedConfig()` — returns the canonical value, else the alias with a
one-time `logger.warn` naming the canonical key (graph-lint can't reach flows
already stored in prod that are never re-published; the warn covers that gap).
- wire it into get/create/update/delete_record for `objectName`/`filter`.
- fix the one authoring source that taught the alias: skills/objectstack-automation
SKILL.md update_record example `object:` -> `objectName:`.
No alias is straight-removed — all are plausibly present in stored prod flows.
Removal is a tracked follow-up gated on the warn window + a cloud graph-lint rule.
Siblings (notify to/subject/body, script inputs/function, wait duration/signal,
assignment name/key, http-dispatcher trigger `object`) get the same treatment in
follow-ups.
Tests: service-automation 225 pass (incl. new crud-config-aliases.test.ts), spec
6600 pass, tsup DTS build clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercelBot commented Jun 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
specReadyReadyPreview, CommentJun 28, 2026 11:00am

Request Review

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests size/m labels Jun 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): packages/services.

5 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/guides/packages.mdx(via packages/services)
  • content/docs/guides/runtime-services/audit-service.mdx(via packages/services)
  • content/docs/guides/runtime-services/index.mdx(via packages/services)
  • content/docs/guides/runtime-services/settings-service.mdx(via packages/services)
  • content/docs/protocol/objectos/i18n-standard.mdx(via packages/services)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review June 28, 2026 13:23
@os-zhuang
os-zhuang merged commit de01520 into mainJun 28, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/sad-yonath-1f92c5 branch June 28, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mtests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@os-zhuang