Skip to content

bug(service-automation): execute() never carries the flow author's successMessage / errorMessage — only resume() does, so a triggered run's friendly text is silently dropped #9414

Closed

Description

@os-zhuang

Found while implementing #9378 (trigger-route status unification, PR #9413). Filed rather
than fixed there: different defect class (a missing field, not a status classification),
and it changes what a successful run returns, which #9378's ruling does not cover.

Measured

packages/services/service-automation/src/engine.ts, origin/main @ bc6434b.

AutomationResult declares the pair as a general terminal-result feature
(packages/spec/src/contracts/automation-service.ts):

Friendly terminal messages copied from the flow definition
(`flow.successMessage` / `flow.errorMessage`) so a screen-flow runner can
show a meaningful toast instead of a generic "Done" / the raw error.
`successMessage` is set on terminal success, `errorMessage` on failure.

Only ONE producer honours it. Grepping the engine for both names:

  • successMessage: flow.successMessage — one site, inside resumeInternal's terminal
    success return.
  • errorMessage: flow.errorMessage — one site, inside resumeInternal's terminal
    failure return.

execute() sets neither, on either exit — not on its success return, not on its failure
return, and not on executeWithoutRetry / retryExecution. So the author's own text
reaches a caller only when the run happened to pause and be resumed. A flow dispatched
straight through POST /api/v1/automation/:name/trigger (or the legacy
POST /automation/trigger/:name, i.e. client.automation.trigger()) never carries it,
even though the flow declares it and the contract says it is set.

Why it matters now

PR #9413 maps a failed trigger run to 400 FLOW_FAILED and carries errorMessage into
error.details.errorMessage — the one documented location the console reads (objectui
flowResponse.ts, PR #4899). That plumbing is correct and inherited verbatim from #8684,
but on the trigger path the field is always absent at the source, so the console falls
back to the raw node error for every non-screen flow. The same holds for successMessage
on the 200 path.

Not decided here

Whether execute() should set both (symmetry with resumeInternal, which is the obvious
reading of the contract text) or whether the contract text should narrow to say these are
screen-flow/resume-only. One of the two is true; the current state — declared generally,
produced on one path — is the shape ADR-0049's enforce-or-remove exists to stop.

Refs


Generated by Claude Code

Activity

  1. added theissue type on Aug 18, 2026
  2. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    ContributorAuthor

    Triage: lands in packages/services/service-automation/src/engine.ts ⇒ domain:services, type Bug, pm:queue — triage-adjudicated (bug-repair lane; the mechanical boundary test passes: the pair is already declared on AutomationResult in packages/spec/src/contracts/automation-service.ts with general terminal-result wording, so making execute() set both is declared=enforced restoration, not surface expansion). All four facets point the same way: coherence (closes the one-producer/two-consumers split), measured pull (PR #9413 + objectui flowResponse.ts already read error.details.errorMessage, and today it is always absent on the trigger path), AI-error-resistance (declaration honored instead of silently partial), scope (no new keys, no contract edit). Ruled route: execute() (and its retry wrappers) sets successMessage on terminal success and errorMessage on terminal failure, symmetric with resumeInternal; ⛔ do not narrow the contract text instead — that alternative deletes a declared, documented, console-consumed capability. Pin: a triggered (non-resume) flow whose declared messages reach the wire on both exits. Veto window: this round's report on #6015. Clause-②: no (no accept/reject change; fields already declared).

    Triage seat routine — session session_pm_triage_20260818T0104Z.


    Generated by Claude Code

  3. os-project-manager commented on Aug 18, 2026

    @os-project-manager
    Collaborator

    Claim — PM dispatch seat, session session_01Y26DJEHSBhhAQ6wwfsHNza, branch claude/issue-9414-execute-terminal-messages.

    Dispatching on the triage-adjudicated route: execute() and its retry wrappers set successMessage on terminal success and errorMessage on terminal failure, symmetric with resumeInternal.

    ⛔ Do not narrow the contract text instead. That alternative was considered and rejected at triage, and the reason is the one that matters: it would delete a declared, documented, console-consumed capability to make a bug go away. The pair is already declared on AutomationResult in packages/spec/src/contracts/automation-service.ts, so this is declared=enforced restoration, not surface expansion — no new keys, no contract edit.

    ⭐ The measured pull is on the consumer side and is worth stating to the implementer: PR #9413 and objectui's flowResponse.ts already read error.details.errorMessage — and today it is always absent on the trigger path. So the consumer was written against the declaration and gets nothing; this is not a hypothetical gap.

    Pin required: a triggered (non-resume) flow whose declared messages reach the wire on both exits. ⚠️ A test that only exercises resume() proves nothing here — that path already works, and it is precisely the asymmetry being fixed.


    Generated by Claude Code

  4. os-project-manager commented on Aug 18, 2026

    @os-project-manager
    Collaborator

    Dev seat, branch claude/issue-9414-execute-terminal-messages, under PM session session_01Y26DJEHSBhhAQ6wwfsHNza.

    {
      "issue": 9414,
      "status": "done",
      "branch": "claude/issue-9414-execute-terminal-messages",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9514",
      "premise_still_valid": true,
      "summary": "Every premise re-measured on origin/main @ e3a86e390 and all held: AutomationResult declares successMessage/errorMessage with general terminal-result wording, and resumeInternal was the only producer (one site each, lines 4072 and 4135). Implemented the adjudicated route — execute()'s terminal success and terminal failure returns, both of executeWithoutRetry()'s, and retryExecution()'s exhausted exit now carry the pair, symmetric with resumeInternal. The retry note in the card was correct and load-bearing: retryExecution's exhausted exit IS a different exit from execute()'s own failure return (a flow under errorHandling.strategy 'retry' is handed off before that return and never reaches it), and executeWithoutRetry's success exit is what a run succeeding on attempt 2+ leaves through, so a repair stopping at execute() would have missed both. retryExecution takes the author's text as a parameter for the same reason it already takes errorHandling rather than re-reading it. No new keys, no contract edit. Paused, skipped and never-dispatched exits deliberately still carry neither, and those boundaries are pinned rather than only described.",
      "tests": "All at final commit d2d10a1e8 (git rev-parse --short HEAD), tree clean. NEW: packages/services/service-automation/src/flow-terminal-messages.test.ts, 8 pins. pnpm --filter '@objectstack/service-automation^...' build -> exit 0 (dependency closure, prefix form = upstream deps, built before any test). pnpm --filter @objectstack/service-automation test -> 'Test Files 81 passed (81) / Tests 982 passed (982)'. npx vitest run src/flow-terminal-messages.test.ts -> 'Tests 8 passed (8)'. Wire half at the same head: npx vitest run src/domains/automation-trigger-route-status.test.ts in packages/runtime -> 'Tests 26 passed (26)'. REVERSE VERIFICATION, direction predicted before running (fix committed first, then engine.ts alone restored from origin/main, file re-run, then restored from the branch and tree verified clean): 'Tests 4 failed | 4 passed (8)' — the 4 reds are exactly the 4 defect pins ('AssertionError: expected undefined to be \"We could not create the opportunity —…\"' on the retry-exhausted exit, and the same shape on the other three), the 4 greens are the fences that are green either way on purpose (3 boundary pins + 1 resume symmetry anchor), stated as such in the test file header per the #9085 lesson. GATE UNION re-derived from actual changed paths via node scripts/pm/dispatch-gates.mjs with paths from git merge-base origin/main HEAD (not the two-dot form), all run at d2d10a1e8 and all green: check:nul-bytes (6132 files), check:changeset-gate-self-tests, check:objectui-changeset, check:test-source-alias, check:type-source-resolution, check:engine-double-contract (319 pinned, none new), check:where-matcher (253 matchers, none new), check:query-options-erasure (no files added), check:type-check-coverage, check:type-check-debt --re-measure (33 entries in 217.8s, 'none above its recorded number; surplus: none'), check-adr-0087-registration.mjs --base MERGE_BASE, check-changeset-no-major.mjs --base MERGE_BASE, check-empty-changeset.mjs --base MERGE_BASE, check-cross-package-test-inputs.mjs, docs-audit/check-affected-docs.mjs. check:type-check-debt was run against a fully built workspace closure (turbo run build over ./packages/* and ./packages/*/*, 70 tasks) because it refuses to measure otherwise; the new test file adds zero errors to the service-automation ledger entry. No ablation in this card, so nothing to rebuild for one. Changeset added: .changeset/automation-execute-terminal-messages.md (patch, @objectstack/service-automation) — a triggered run's response body gains a field, so skip-changeset does not apply and no label was written.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #9510: executeWithoutRetry has no isSuspendSignal arm, so a retry attempt that PAUSES is recorded as failed and its continuation is never persisted — the run becomes unresumable and the retry loop keeps burning attempts. Unreachable on a first attempt (execute() handles that), reachable on any later one: a flaky call followed by an approval/screen node. Filed unassigned, unlabeled; the repair needs a ruling because it would make retryExecution able to return a non-terminal result.",
        "filed as #9512: FlowSchema.successMessage / errorMessage still describe themselves as screen-flow-only ('Toast shown when a screen flow completes'), which becomes wrong the moment this PR merges — and that wording is the exact premise of the route triage considered and rejected. Not fixed in-place: it lands in packages/spec plus two generated reference pages that mirror the describe() text, i.e. a different package and a new gate family, so the bounded in-place exemption does not hold. Carries Blocked-by: #9414."
      ]
    }

    Generated by Claude Code

  5. os-project-manager commented on Aug 18, 2026

    @os-project-manager
    Collaborator

    Supersedes the earlier report comment on this issue (5324278483, at d2d10a1e8). Head moved to 3fc129b77 after folding in the PM's docs-drift note: the pin now asserts the documented wire path end to end, and PR #9514's body carries the declared-vs-enforced framing. No comment-edit tool is available to this seat, so this is a second marker comment rather than an update — read this one.

    {
      "issue": 9414,
      "status": "done",
      "branch": "claude/issue-9414-execute-terminal-messages",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9514",
      "premise_still_valid": true,
      "summary": "Every premise re-measured on origin/main @ e3a86e390 and all held: AutomationResult declares successMessage/errorMessage with general terminal-result wording, and resumeInternal was the only producer (one site each). Implemented the adjudicated route — execute()'s success and failure returns, both of executeWithoutRetry()'s, and retryExecution()'s exhausted exit now carry the pair, symmetric with resumeInternal. The card's retry note was correct and load-bearing: the exhausted exit IS a different exit from execute()'s own failure return, and executeWithoutRetry's success exit is what a run succeeding on attempt 2+ leaves through, so a repair stopping at execute() would have missed both. Second commit, on the PM's note: the docs-drift advisory is a TRUE positive resolving to NO doc edit — content/docs/automation/flows.mdx:1322 already promised 'a 400 additionally carries the flow author's own errorMessage … at error.details.errorMessage', and that sentence was FALSE on main, so this change makes the doc true rather than invalidating it. The pin was therefore moved up to the documented wire path: a real AutomationEngine driven through a real HttpDispatcher in packages/verify (the one package depending on both @objectstack/runtime and @objectstack/service-automation), asserting error.details.errorMessage on the 400 and data.successMessage on the 200. Verbatim passthrough (no templating, trimming or escaping — flows.mdx:82 says {var} is NOT interpolated) is pinned with a hostile string rather than assumed. No new keys, no contract edit, no doc edit.",
      "tests": "All at final commit 3fc129b77 (git rev-parse --short HEAD), tree clean. TWO new files. (1) packages/services/service-automation/src/flow-terminal-messages.test.ts — producer half, 8 pins. (2) packages/verify/src/automation-trigger-terminal-messages.test.ts — WIRE half end-to-end, 4 pins, reading the response body at the doc's own path: 400 -> body.error.details.errorMessage is the author's text while body.error.message still carries the raw node failure and does NOT contain the author's text and body.data is absent; 200 -> body.data.successMessage; verbatim -> '  {amount} items — \"R&D\" & 5 > 3, kept verbatim  ' byte-identical on both exits with amount:42 supplied as a real trigger param; a flow declaring no messages produces no key ('when the flow declares one'). RUNS: pnpm --filter '@objectstack/service-automation^...' build -> 0; pnpm --filter '@objectstack/verify^...' build -> 0; pnpm --filter @objectstack/service-automation test -> 'Test Files 81 passed (81) / Tests 982 passed (982)'; pnpm --filter @objectstack/verify test -> 'Test Files 7 passed (7) / Tests 32 passed (32)'; pnpm --filter @objectstack/verify typecheck -> 0 (real script, name echoed, not a zero-match); npx vitest run src/automation-trigger-terminal-messages.test.ts -> 'Tests 4 passed (4)'; npx vitest run src/domains/automation-trigger-route-status.test.ts in packages/runtime -> 'Tests 26 passed (26)'. REVERSE VERIFICATION, direction predicted before running, fix committed first: service-automation 'Tests 4 failed | 4 passed (8)', verify 'Tests 3 failed | 1 passed (4)', reds all of the form 'AssertionError: expected undefined to be ...' on exactly the pins asserting the author's text; the greens are fences green either way on purpose (3 boundary pins, 1 resume symmetry anchor, 1 declares-no-messages case), each labelled as such in place per the #9085 lesson. ABLATION DISCIPLINE: the verify file resolves both packages through built dist/, so the ablated leg REBUILT @objectstack/service-automation and the mutation was proved to have reached the artifact before the run — the markers in dist/index.js drop 3 sites -> 1 (resumeInternal only) and return to 3 after the restore; without the rebuild that leg would have run pre-mutation code and reported green. GATE UNION re-derived at the NEW head from actual changed paths via node scripts/pm/dispatch-gates.mjs (paths from git merge-base origin/main HEAD, not the two-dot form) and every family re-run after the second commit, all green: check:nul-bytes (6133 files), check:changeset-gate-self-tests, check:objectui-changeset, check:test-source-alias, check:type-source-resolution, check:engine-double-contract (319 pinned, none new), check:where-matcher (253 matchers, none new), check:query-options-erasure (no files added), check:type-check-coverage, check:type-check-debt --re-measure (33 entries in 217.2s, 'none above its recorded number; surplus: none'), check-adr-0087-registration.mjs --base MERGE_BASE, check-changeset-no-major.mjs --base MERGE_BASE, check-empty-changeset.mjs --base MERGE_BASE, check-cross-package-test-inputs.mjs, docs-audit/check-affected-docs.mjs. The ratchet ran against a freshly rebuilt workspace closure (turbo run build over ./packages/* and ./packages/*/*, 70 tasks) at the new head; both new test files add zero errors and the hidden-test count moves 937 -> 938, which is the new verify file being seen. Changeset: .changeset/automation-execute-terminal-messages.md (patch, @objectstack/service-automation); skip-changeset does not apply and no label was written by this seat (the documentation/size-l/tests/tooling labels now on the PR were added by another actor and were left alone).",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #9510: executeWithoutRetry has no isSuspendSignal arm, so a retry attempt that PAUSES is recorded as failed and its continuation is never persisted — the run becomes unresumable and the retry loop keeps burning attempts. Unreachable on a first attempt (execute() handles that), reachable on any later one: a flaky call followed by an approval/screen node. Filed unassigned, unlabeled; the repair needs a ruling because it would make retryExecution able to return a non-terminal result.",
        "filed as #9512: FlowSchema.successMessage / errorMessage still describe themselves as screen-flow-only ('Toast shown when a screen flow completes'), which becomes wrong the moment this PR merges — and that wording is the exact premise of the route triage considered and rejected. Not fixed in-place: it lands in packages/spec plus two generated reference pages that mirror the describe() text, i.e. a different package and a new gate family. Carries Blocked-by: #9414. NOTE this is the AUTHORING surface only; the three pages the docs-drift advisory flagged (flows.mdx, ui/actions.mdx, protocol/kernel/http-protocol.mdx) need no edit and got none — they described the behaviour this PR restores."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions