Skip to content

fix(showcase): unstrand the Invoice Dual Sign-off approval demo, and the second instance of the same hop (#7381) - #7395

Merged
os-help merged 1 commit into
mainfrom
claude/issue-7381-showcase-approval-expand
Aug 10, 2026
Merged

fix(showcase): unstrand the Invoice Dual Sign-off approval demo, and the second instance of the same hop (#7381)#7395
os-help merged 1 commit into
mainfrom
claude/issue-7381-showcase-approval-expand

Conversation

@os-help

@os-help os-help commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7381

Approving Invoice Dual Sign-off in the showcase recorded the decision and then stranded the flow run, so the "Notify: Cleared" inbox message the demo promises never arrived. notify_cleared addressed {record.account.owner} while the start node declared no config.expand, the hop read a scalar foreign key, and the notify node refuses a run with zero recipients.

The triage-suggested repair does not work, and the measurement says why

The card preferred option 1 — keep the hop, add expand: ['account']. Measured against the schema, that leaves the demo just as broken: showcase_account has no owner field. Its declared fields are name, industry, annual_revenue, website, hq, status, sales_region, signed_on, tax_id, support_config, churn_reason, billing_email; the platform's injected record-owner column is owner_id, not owner (OWNER_COLUMN, packages/spec/src/data/injected-system-columns.ts). So account.owner is undefined however thoroughly the relation is hydrated, and the run strands identically.

That is asserted rather than argued — one test reads the persisted request snapshot and pins that the hydrated account has no owner key and does have billing_email.

What landed instead

Both halves of option 1's intent, with a field that exists:

  • Recipient goes to {record.owner}, the invoice's own owner. It is the seeded rep, and the same anchor the showcase_contributor permission set scopes invoices by — which is who "your invoice cleared sign-off" is addressed to anyway.
  • Hydration kept and made live — the start node declares expand: ['account'] and the message body reads {record.account.name}. The flagship demo still teaches the config.expand path; the declaration is not inert config.

Why the pause matters: expandDeclaredLookups runs once at flow START, but the failing read happens after an approval that can sit for days. It works because the expander mutates the run's record in place, so the hydrated relation is part of the state persisted at suspend and restored verbatim by resumeInternal.

Sweep — the card's completion requirement

Every {a.b.c} relation-hop read across all showcase flow files (comment lines excluded), against whether its start node declares a matching expand. 29 flows swept; 27 make no relation hop at all and need none.

Flow Relation-hop reads expand before expand after Disposition
showcase_invoice_signoff {record.account.owner}, now {record.account.name} none ['account'] The reported defect. Hop was unfixable as written (no account.owner); recipient re-pointed at {record.owner}, hop kept for the account name.
showcase_task_done_notify_owner {record.project.owner} into the subflow's {ownerId} then notify none ['project'] Second live instance, found by this sweep. Hop is sound — showcase_project.owner is a real seeded field — so declaring the relation is the whole fix.
other 27 flows none none none No relation hop; nothing to declare.

The second instance was not theoretical: on origin/main every completion of a task ran showcase_task_done_notify_owner to a failure, and the reverse check below reproduces it independently. It is fixed here per the card's "same class, same file face" clause.

One comment-only clarification on showcase_task_completed, which deliberately uses {record.assignee}: its note said the hop "would resolve to an empty string" without naming the remedy, which now reads as though hops are impossible.

Tests

New real-kernel suite, examples/app-showcase/test/approval-resume-relation-expand.test.ts — a real ObjectKernel with the real ObjectQL / automation / approvals / messaging plugins over real sqlite, the app's real objects and real flow definitions. Requests are opened the way registerShowcaseApprovalDemo does and decided through ApprovalService.decide with the position: actor ids the flow's own docblock documents. This is the full level the card asked for, not the reduced fallback.

Four cases: the end-to-end approve to resume to delivered inbox message; the persisted-snapshot hydration pin; an in-test rollback to the pre-fix shape; and the sweep's second instance.

Test Files  17 passed (17)
      Tests  168 passed (168)          # full showcase suite

Reverse verification — direction predicted, then measured

The fix was taken out at source (git checkout origin/main -- …flows/index.ts, never git stash) and the same suite re-run. Predicted direction: red on resume, with the run stranded while the decision still lands. Measured, 3 of 4 red:

× satisfies both unanimous slots and runs through to end_ok with the inbox message delivered
Error: RESUME_FAILED: the approve decision was recorded on request areq_8222434a-…, but its
flow run 'run_29f74521-…' could not be resumed and is now stranded: resume of run … failed:
Node 'notify_cleared' failed: notify: at least one recipient is required, but every recipient
template resolved to nothing: {record.account.owner}.

× persists the invoice with its account HYDRATED, which is what survives the pause
AssertionError: the relation was stored as a scalar id, not hydrated: expected 'string' to be 'object'

× sweep: the task-done subflow resolves the project owner through the declared expand
Node 'call_notify' failed: subflow 'showcase_notify_owner' failed: Node 'notify' failed:
notify: at least one recipient is required, but every recipient template resolved to nothing: {ownerId}.

The first reproduces #7381's reported toast verbatim. The second matches the issue's quoted payload ("account": "h8AhbJyB-…", a bare id) — post-fix that column stores the hydrated object. The fourth case stays green by design: it authors the pre-fix shape itself, so the source revert cannot change it.

Rejection-class note: the pre-fix case asserts the diagnostic's content (notify_cleared, the empty template, expand), not a bare throw — a throw-only assertion would stay green on any unrelated failure.

Gates

  • pnpm --filter @objectstack/example-showcase test — 168 passed
  • pnpm --filter @objectstack/example-showcase typecheck — clean
  • pnpm --filter @objectstack/example-showcase validate — no errors (warnings pre-existing; expand raises no unrecognized_keys)
  • eslint on both touched files — clean
  • node scripts/check-nul-bytes.mjs — OK

The harness needed four devDependencies the showcase did not carry (@objectstack/core, service-automation, service-messaging, plugin-approvals) — dev-only, same pattern as examples/app-todo.

Changeset: patch for @objectstack/example-showcase.


Generated by Claude Code

`notify_cleared` addressed `{record.account.owner}` with no `expand` on the
start node, so the hop read a scalar FK and resolved to nothing. The notify
node refuses a run with zero recipients, so approving the demo recorded the
decision and stranded the flow run at its payoff moment.

The hop was unfixable as written: `showcase_account` has no `owner` field.
The notice now addresses `showcase_invoice.owner`, while the start node
declares `expand: ['account']` and the body reads `{record.account.name}`, so
the demo still teaches relation hydration with a field the account has.

Sweep: `showcase_task_done_notify_owner` carried the same resume-time pattern
(`{record.project.owner}` into a subflow notify, no expand) and is fixed too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 9:21am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

  • Temporal Conformance (live PG + MySQL) — 失败步骤: Install dependencies(日志不可读,点进 job 看)
  • Build Docs — 失败步骤: Install dependencies(日志不可读,点进 job 看)
  • Dogfood Verify CLI — 失败步骤: Install dependencies(日志不可读,点进 job 看)
  • Build Core — 失败步骤: Install dependencies(日志不可读,点进 job 看)

历史信号:

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

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

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

Merged via the queue into main with commit 69f1a5f Aug 10, 2026
26 checks passed
@os-help
os-help deleted the claude/issue-7381-showcase-approval-expand branch August 10, 2026 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants