Skip to content

fix(cli): make os explain flow teach a flow that actually parses - #14809

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-14782-explain-flow-currentuser-example
Sep 3, 2026
Merged

fix(cli): make os explain flow teach a flow that actually parses#14809
os-project-manager merged 3 commits into
mainfrom
claude/issue-14782-explain-flow-currentuser-example

Conversation

@os-trump

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

Copy link
Copy Markdown
Collaborator

Fixes #14782

What the card asked, and what measuring found

The card said os explain flow's example teaches $currentUser, a spelling nothing
resolves. Triage added a ⛔: settle the sample's node SHAPE before touching the token,
because "correcting $currentUser in a sample whose shape is also wrong ships a second
wrong 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 Flow at all. Measured against the real FlowSchema
(built @objectstack/spec, origin/main source read via git show):

--- ORIGIN/MAIN example ---
{ name: 'assign_on_create', type: 'autolaunched', label: 'Auto-Assign on Create',
  trigger: { object: 'project_task', event: 'afterInsert' },
  steps: [ { type: 'assignment', field: 'assigned_to', value: '$currentUser' } ] }
--- PARSE SUCCESS: false
  ISSUE [nodes] Invalid input: expected array, received undefined
  ISSUE [edges] Invalid input: expected array, received undefined
  ISSUE [] Unrecognized key(s) on this flow: `trigger`, `steps`.
           Did you mean `trigger` -> `type`, `steps` -> `nodes`?

So the token was the third defect, not the only one:

  1. steps and trigger are strict-object aliases on FlowSchema (for nodes and
    type). A record-change flow binds its object on the START node's config
    ({ objectName, triggerType, condition }), never at the flow top level — the schema's
    own guidance block says so.
  2. 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 absent. edges is required; the sample declared no graph.
  3. '$currentUser' is a sentinel no resolver recognises.

A fourth, which triage flagged as unproven and I can now confirm the shape of: an
assignment node calls variables.set — it sets a flow variable, not a record field.
"Auto-assign on create" is an update_record node. The old sample would not have written
assigned_to even with a resolving token.

The same package already knew

os generate flow scaffolds the correct shape today — packages/cli/src/commands/generate.ts
emits type: 'record_change', status, nodes with a start node carrying
config: { objectName, triggerType }, and edges. Its own comment says it outright:

A record-change flow binds its trigger HERE, on the START node's config — there is no
top-level trigger key.

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 flow
and os generate flow now 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 -F flag.

Negative — git grep -n -F '$currentUser', whole repo, 4 hits, zero in any resolver:

docs/adr/0017-object-has-many-view.md:216   SQL-shaped prose
packages/cli/src/commands/explain.ts:128    -- this card
skills/objectstack-ui/rules/list-views.md:95    -- PR #14781's lane, NOT touched here
skills/objectstack-ui/rules/list-views.md:103   -- PR #14781's lane, NOT touched here

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 own
doc line and two shipping example apps:

packages/services/service-automation/src/builtin/template.ts:11
   *   {$User.Id}                -> reads from context.userId
content/docs/automation/flows.mdx:1438       the value-dialect vocabulary table
examples/app-showcase/src/automation/flows/index.ts:1709
examples/app-todo/src/flows/task.flow.ts:320
skills/objectstack-automation/SKILL.md:110, :289
packages/lint/src/lint-flow-patterns.ts:1535 the lint hint text

Executable control — what the resolver actually returns

Source reading is not proof, so I ran the real interpolate (the exact function
logic-nodes.ts:135 and the CRUD nodes call) over all five spellings in one run, with two
known-good tokens as in-run positive controls:

authored value interpolate(...) returns
'$currentUser' "$currentUser" — stored verbatim as a literal string
'{$currentUser}' undefined
'{current_user_id}' undefined
'{$User.Id}' control "usr_alice" (the context's userId)
'{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 bogus
literal landing in a lookup('sys_user') column, which is the failure class the card feared.

⚠️ The two doors resolve DIFFERENT tokens — deliberately

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} returns
undefined on this surface.

This is by design, not drift. template.ts interpolateFilter documents it outright — a
filter value position is "the one place where two {...} dialects meet", and the filter
placeholder vocabulary is handed to the query engine only there. Assignment values and
update_record fields go through plain interpolate, 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:

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/cli plus the changeset. No sibling repo, no
content/docs/releases/.

  • packages/cli/src/commands/explain.ts — the flow catalog entry: required /
    optional field lists realigned to FlowSchema (nodes, edges, the full five-value
    type enum, status, runAs; steps and trigger removed as the non-keys they are),
    and the example rewritten to a flow that parses.
  • packages/cli/test/commands.test.ts — four pins in the existing
    os explain — schema catalog accuracy block, which already exists for exactly this
    hazard (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 guard
that 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 closure
was built first (pnpm --filter '@objectstack/cli^...' build,
os-verify-lock: VERDICT command-exit 0), so all of it reads rebuilt dist, never cache.
Exit codes are captured before any pipe, and each line quotes the gate's own verdict text.

Testsos-verify-lock: VERDICT command-exit 0:

pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 test/commands.test.ts
 Test Files  1 passed (1)
      Tests  18 passed (18)          -- 14 pre-existing + the 4 pins added here

pnpm --filter @objectstack/cli typecheck
> tsc --noEmit                        (no diagnostics)

Reverse verification — both readings through the same built FlowSchema:

example source FlowSchema.safeParse
origin/main false — the three issues quoted at the top
this branch true

⚠️ One NOT MEASURED, stated rather than counted as green. packages/cli/tsconfig.json
declares include: ['src'], so tsc --noEmit never reads packages/cli/test/. The clean
typecheck above is a true statement that says nothing about the test file I edited. This is
pre-existing and ledgered — check:type-check-coverage reports "12 package(s) still hide
their 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/main version of the same file as the baseline (swapped in place, restored by
git checkout HEAD -- ... and the restore confirmed by blob-hash equality, not by an exit
code):

TS2835 TS7006 TS18046 total
origin/main baseline 13 2 0 15
first draft of the pins 13 4 1 18
as committed 13 2 0 15

The 15 are pre-existing (extensionless relative imports, which under NodeNext make
SCHEMAS widen to any). The pins now add zero. That mattered here rather than being
tidiness: an implicit any in a pin makes the assertion stop checking anything, in a file
no 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-major

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack derives 37
families 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

  • Did not touch skills/objectstack-ui/rules/list-views.md. Those two $currentUser
    hits 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.
  • Did not touch docs/adr/0017-object-has-many-view.md:216. That hit is SQL-shaped
    prose (scope='shared' OR owner = $currentUser), not an authoring sample — a different
    falsehood class, if it is one at all.
  • Did not invent or add an acting-user token, and did not borrow current_user from
    the default-value surface. A resolving spelling already exists here ({$User.Id}), so
    triage's "no such token" branch never applied.
  • Did not open an ADR for the token vocabulary, per triage.
  • Did not audit the other eleven catalog entries. Only the flow entry is in this
    card's face. The parse-the-example guard is deliberately written for flow alone rather
    than 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

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
`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
@github-actions github-actions Bot added the size/m label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 8 documentable anchor(s).

42 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 5a5336b399db2ef18dd4700f97d579a328197dda.

6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 5 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 22 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 5a5336b399db2ef18dd4700f97d579a328197ddapackageMentionDocs.

Which tree this was computed on

This run read content/docs from ac6d1affefe53cff7f3fd7aaca955f20dba5b6fb — the merge of head 2ae166740f83a292278ee7e78707eb092e7cf98c into base 5a5336b399db2ef18dd4700f97d579a328197dda, 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 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

⚠️ 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 5a5336b399db2ef18dd4700f97d579a328197dda → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33716744532 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test:  FAIL   integration  test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
      ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1800
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 69 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33718085200 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test:  FAIL   integration  test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
      ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • ⚠️ 本 PR 过去 24h 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 71 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

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

Development

Successfully merging this pull request may close these issues.

os explain flow's example teaches $currentUser as an assignment value, a spelling nothing resolves

3 participants