fix(cli): make os explain flow teach a flow that actually parses - #14809
Conversation
The catalog entry is hand-maintained and does not derive from FlowSchema,
so its sample drifted into teaching a shape the spec rejects outright:
- `steps` and `trigger` are strictObject ALIASES on FlowSchema (for
`nodes` and `type`). Authoring either is a loud parse error, and a
record-change flow binds its object on the START node's `config`,
not at the flow top level.
- A node's per-type data lives under `config`, so the sample's
top-level `field`/`value` pair were undeclared keys on a `.strict()`
node schema, and the required `id`/`label` were missing.
- `edges` is required; the sample had no graph at all.
- The assignment value `'$currentUser'` is a `$`-prefixed sentinel no
resolver in this repo recognises. The flow value dialect is
brace-based and the acting user is `{$User.Id}`.
Also: an `assignment` node sets a flow VARIABLE, not a record field, so
"assign on create" is an `update_record` node. The sample now shows the
real shape end to end and is pinned by a test that parses it against
FlowSchema, which is the only guard that cannot itself drift.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…le fix Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
`packages/cli/test/commands.test.ts` sits outside every tsc program in the repo (the TEST_DEBT ledger records the package), so nothing would have reported an implicit `any` in the pins added for #14782 — and an implicit `any` there silently stops the assertion from checking anything. Measured with an ad-hoc strict pass over the file: origin/main carries 15 errors (13 TS2835 from its extensionless relative imports, 2 TS7006 in the pre-existing ownership test). The first draft of the pins took that to 18. With a local `CatalogField` shape and a typed `Object.entries` cast it is back to exactly the baseline 15 — no new error, and no widening of what the pins actually assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
📓 Docs Drift CheckThis PR changes 1 package(s): 42 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 22 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 ac6d1affefe53cff7f3fd7aaca955f20dba5b6fb && git checkout ac6d1affefe53cff7f3fd7aaca955f20dba5b6fb
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5a5336b399db2ef18dd4700f97d579a328197dda 2ae166740f83a292278ee7e78707eb092e7cf98c && git checkout -B drift-repro 5a5336b399db2ef18dd4700f97d579a328197dda && git merge --no-ff 2ae166740f83a292278ee7e78707eb092e7cf98c
node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda
|
Fixes #14782
What the card asked, and what measuring found
The card said
os explain flow's example teaches$currentUser, a spelling nothingresolves. Triage added a ⛔: settle the sample's node SHAPE before touching the token,
because "correcting
$currentUserin a sample whose shape is also wrong ships a secondwrong sample that now looks authoritative."
Triage offered two branches. Neither holds — the answer is a third and worse one.
The sample does not parse as a
Flowat all. Measured against the realFlowSchema(built
@objectstack/spec,origin/mainsource read viagit show):So the token was the third defect, not the only one:
stepsandtriggerare strict-object aliases onFlowSchema(fornodesandtype). A record-change flow binds its object on the START node'sconfig(
{ objectName, triggerType, condition }), never at the flow top level — the schema'sown
guidanceblock says so.config, so the sample's top-levelfield/valuepair were undeclared keys on a
.strict()node schema, and the requiredid/labelwere absent.
edgesis required; the sample declared no graph.'$currentUser'is a sentinel no resolver recognises.A fourth, which triage flagged as unproven and I can now confirm the shape of: an
assignmentnode callsvariables.set— it sets a flow variable, not a record field."Auto-assign on create" is an
update_recordnode. The old sample would not have writtenassigned_toeven with a resolving token.The same package already knew
os generate flowscaffolds the correct shape today —packages/cli/src/commands/generate.tsemits
type: 'record_change',status,nodeswith a start node carryingconfig: { objectName, triggerType }, andedges. Its own comment says it outright:So one CLI command was scaffolding the truth while its sibling explained the opposite. The
replacement sample is deliberately the same shape as that scaffold, so
os explain flowand
os generate flownow agree.Grep readings, with the positive control
The load-bearing claim is a negative ("nothing resolves it"), so a zero-result grep
alone proves nothing. Both controls are run with the same tool and the same
-Fflag.Negative —
git grep -n -F '$currentUser', whole repo, 4 hits, zero in any resolver:The pattern itself is proven live by returning 4 hits, so a zero-match on any narrower
form of it is a real absence rather than a broken pattern.
Positive control —
git grep -n -F '{$User.Id}', 17 hits, including the resolver's owndoc line and two shipping example apps:
Executable control — what the resolver actually returns
Source reading is not proof, so I ran the real
interpolate(the exact functionlogic-nodes.ts:135and the CRUD nodes call) over all five spellings in one run, with twoknown-good tokens as in-run positive controls:
interpolate(...)returns'$currentUser'"$currentUser"— stored verbatim as a literal string'{$currentUser}'undefined'{current_user_id}'undefined'{$User.Id}'control"usr_alice"(the context'suserId)'{record.id}'control"rec_1"This settles the half the card explicitly left unproven.
'$currentUser'is not rejected —it passes through as text. In a
fields: { assigned_to: ... }position that is a bogusliteral landing in a
lookup('sys_user')column, which is the failure class the card feared.My dispatch asked me to make this sample and sibling PR #14781 teach ONE spelling. They
cannot, and should not. Row three above is the measurement:
{current_user_id}returnsundefinedon this surface.This is by design, not drift.
template.tsinterpolateFilterdocuments it outright — afilter value position is "the one place where two
{...}dialects meet", and the filterplaceholder vocabulary is handed to the query engine only there. Assignment values and
update_recordfieldsgo through plaininterpolate, where a bare{current_user_id}is "a nonsense reference rather than a query bound."
So the correct answer is per-surface and both siblings are right:
{current_user_id}(PR fix(skills): teach {current_user_id}, not $currentUser, in the objectstack-ui Filtering example #14781, unchanged by me){$User.Id}(this PR)Flagged in the report rather than silently picked. No card filed: this is a documented,
intentional split, not a defect.
File face
Three files, all in
packages/cliplus the changeset. No sibling repo, nocontent/docs/releases/.packages/cli/src/commands/explain.ts— theflowcatalog entry:required/optionalfield lists realigned toFlowSchema(nodes,edges, the full five-valuetypeenum,status,runAs;stepsandtriggerremoved as the non-keys they are),and the
examplerewritten to a flow that parses.packages/cli/test/commands.test.ts— four pins in the existingos explain — schema catalog accuracyblock, which already exists for exactly thishazard (a hand-maintained catalog that does not derive from the spec).
.changeset/explain-flow-example-parses.md—@objectstack/cli: patch.The strongest pin parses the example against the real
FlowSchema— the one guardthat cannot drift alongside the catalog it checks, because it re-derives the truth from
the spec on every run.
Verification
Every reading below is at head
2ae166740f, working tree clean. The dependency closurewas built first (
pnpm --filter '@objectstack/cli^...' build,os-verify-lock: VERDICT command-exit 0), so all of it reads rebuiltdist, never cache.Exit codes are captured before any pipe, and each line quotes the gate's own verdict text.
Tests —
os-verify-lock: VERDICT command-exit 0:Reverse verification — both readings through the same built
FlowSchema:FlowSchema.safeParseorigin/mainpackages/cli/tsconfig.jsondeclares
include: ['src'], sotsc --noEmitnever readspackages/cli/test/. The cleantypecheck above is a true statement that says nothing about the test file I edited. This is
pre-existing and ledgered —
check:type-check-coveragereports "12 package(s) still hidetheir own tests from tsc" and passes.
Rather than leave it there, I measured the file directly with an ad-hoc strict pass, against
the
origin/mainversion of the same file as the baseline (swapped in place, restored bygit checkout HEAD -- ...and the restore confirmed by blob-hash equality, not by an exitcode):
origin/mainbaselineThe 15 are pre-existing (extensionless relative imports, which under
NodeNextmakeSCHEMASwiden toany). The pins now add zero. That mattered here rather than beingtidiness: an implicit
anyin a pin makes the assertion stop checking anything, in a fileno tsc program reads.
Gates run locally, all green, re-run at the final head:
check:nul-bytes·check:cross-package-test-inputs·check:test-source-alias·check:doc-authoring·check:type-check-coverage·check:changeset-gate-self-tests·check:objectui-changeset·check:pm-half-states·check:undeclared-dep-imports·check:keyed-text-bounds·check-empty-changeset·check-changeset-no-majornode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackderives 37families for this diff. The rest are CI's single run, per the local-scope rule — including
check:type-check-debt --re-measure, which needs the whole built workspace closure.What I did NOT do, and why
skills/objectstack-ui/rules/list-views.md. Those two$currentUserhits belong to PR fix(skills): teach {current_user_id}, not $currentUser, in the objectstack-ui Filtering example #14781 on the filter surface. Triage forbade it; editing them here
would collide with that card.
docs/adr/0017-object-has-many-view.md:216. That hit is SQL-shapedprose (
scope='shared' OR owner = $currentUser), not an authoring sample — a differentfalsehood class, if it is one at all.
current_userfromthe default-value surface. A resolving spelling already exists here (
{$User.Id}), sotriage's "no such token" branch never applied.
flowentry is in thiscard's face. The parse-the-example guard is deliberately written for
flowalone ratherthan looped over every entry — extending it is a separate, larger piece of work and
would fail on entries this card has no mandate to rewrite. Named in the report.
Session (durable copy, survives body edits):
https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
🤖 Generated with Claude Code
https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
Generated by Claude Code