fix(showcase): contain the batch-reminders loop body per iteration - #16355
Merged
os-warren merged 1 commit intoSep 6, 2026
Merged
Conversation
`showcase_batch_reminders` ran its `notify` node (`send_reminder`) bare inside the `loop_tasks` body with no `try_catch` between the loop and it. A `loop` body has no error handling of its own — the container iterates with a bare `await` — and `notify` returns `success: false` when every resolved recipient template is empty. One task with a blank `owner` therefore ended the whole run: every later task was never reminded, and the run summary reported `acted: 0` for work that had happened. Adopt the documented containment spelling from content/docs/automation/flows.mdx §"Per-iteration containment": a `try_catch` inside the body with `send_reminder` moved into its `try` region, and a `catch` of one bare `assignment` node (the shortest handler that parses — `catch.nodes` is `.min(1)`, and an omitted `catch` contains nothing). This is the showcase app's first in-repo hit of the `flow-loop-body-uncontained` warning. Example apps are what AI authors copy from, so the flow now demonstrates the loop container AND the per-iteration guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
os-warren
marked this pull request as ready for review
September 6, 2026 15:18
os-warren
enabled auto-merge
September 6, 2026 15:19
os-warren
deleted the
claude/issue-14622-batch-reminders-per-iteration-containment
branch
September 6, 2026 16:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14622
showcase_batch_reminders(BatchRemindersFlow) ran itsnotifynodesend_reminderbare inside theloop_tasksbody, with notry_catchbetweenthe loop and it. A
loopbody has no error handling of its own — the containeriterates with a bare
await— andnotifyreturnssuccess: falsewhen everyresolved recipient template comes back empty. One task with a blank
ownertherefore ended the whole run: every later task was never reminded, and the run
summary reported
acted: 0for work that had happened.The showcase app is the first in-repo hit of the
flow-loop-body-uncontainedwarning that shipped with #14394 (PR #14617), and example apps are what AI
authors copy from.
What changed
The documented containment spelling from
content/docs/automation/flows.mdxunder "Per-iteration containment", verbatim in shape:
try_catchnode (guard_reminder) inside the loop body;send_remindermoved unchanged into itstryregion;catchof one bareassignmentnode (reminder_failed) — the shortesthandler that parses.
FlowRegionSchema.nodesis.min(1), socatch: {}andcatch: { nodes: [] }are both refused, and an omittedcatchparses whilecontaining nothing.
The flow's docblock and
descriptionnow name the guard, so a reader who copiesthe flow copies the containment with it. The loop container, its
maxIterations, its iterator/index variables and its→ endout-edge areuntouched, so the ADR-0031 loop demonstration the flow existed for is intact —
it now demonstrates the per-iteration guard as well.
Option 2 from the card (keep fail-fast and justify it in
description) was nottaken: a reader who copies the flow does not copy the description.
Measurement — the warning histogram, not the build exit code
flow-loop-body-uncontainedis a warning, sopnpm --filter @objectstack/example-showcase buildexits 0 on both trees. The reading is thehistogram, anchored on the
rule:lines the linter prints:flow-loop-body-uncontainedflow-try-catch-without-catchreact-prop-deprecated(control class)approval-approvers-may-resolve-emptyThe control class is unchanged, and the total moves by exactly one: the
histogram is real, and nothing else in the app moved. Both builds exited 0.
The single before-hit named this flow, by name and not by line number:
Premise re-verified on
origin/mainat3e270d4e2(the #16069 commit), as thetriage comment asked: the rule still fires on this flow. The two other
try_catchnodes in this file belong toResilientSyncFlowandProjectEscalationFlow, not to this one.examples/app-showcase/access-matrix.jsonshows no drift —git statusisclean after both builds. Adding a
try_catchinside a flow body does not movethe authorization snapshot, which is what the card predicted.
Verification
All commands below ran on
f5e6ebf20, the head of this branch.pnpm --filter '@objectstack/example-showcase^...' build— dependency closure,exit 0 (so the measurements above read rebuilt output, not stale
dist/).pnpm --filter @objectstack/example-showcase typecheck—tsc --noEmit,exit 0 (the script name is echoed in the log, so this is not a zero-match
no-op).
pnpm --filter @objectstack/example-showcase exec vitest run --maxWorkers=2—28 test files, 379 tests, all passing.
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands— 32 commands, all run, each exit code captured immediately afterits own redirect. All 32 green.
pnpm check:dual-build-cjs-loadsfirst returned exit 3 PREREQUISITE NOTMET (seven packages had no
dist/); those were built and the gate re-run toa real green, rather than recorded as a pass.
eslint . --no-inline-configscan belongs to CI. Locally the changed file was linted directly — 0 errors,
0 warnings;
--format jsonreports 1 file linted;--print-configresolves a governed config for that path (the instrument distinguishes this
from an ignored file: a control run on an ignored path returns "File ignored
because of a matching ignore pattern"). The narrowing excludes nothing,
because
eslint.config.mjsnever enables type-aware linting for any file(no
parserOptions.project, no typed@typescript-eslintrules — stated andmeasured in that file's own header), so a one-file diff cannot change the
verdict of any file it does not touch.
Changeset
None, and this PR carries
skip-changeset. The only edited file lives inexamples/app-showcase, whosepackage.jsondeclares"private": true(line 6), so nothing is published from this change and there is no version to
bump. That is the rule the changeset job encodes — a PR that releases nothing
takes the label.
Scope
One file, one flow.
packages/services/service-automation/**andpackages/lint/**are untouched, and no other flow in the file was changed.🤖 Generated with Claude Code
Generated by Claude Code