Skip to content

fix(showcase): contain the batch-reminders loop body per iteration - #16355

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-14622-batch-reminders-per-iteration-containment
Sep 6, 2026
Merged

fix(showcase): contain the batch-reminders loop body per iteration#16355
os-warren merged 1 commit into
mainfrom
claude/issue-14622-batch-reminders-per-iteration-containment

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #14622

showcase_batch_reminders (BatchRemindersFlow) 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 comes back 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.

The showcase app is the first in-repo hit of the flow-loop-body-uncontained
warning 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.mdx
under "Per-iteration containment", verbatim in shape:

  • a try_catch node (guard_reminder) inside the loop body;
  • send_reminder moved unchanged into its try region;
  • a catch of one bare assignment node (reminder_failed) — the shortest
    handler that parses. FlowRegionSchema.nodes is .min(1), so catch: {} and
    catch: { nodes: [] } are both refused, and an omitted catch parses while
    containing nothing.

The flow's docblock and description now name the guard, so a reader who copies
the flow copies the containment with it. The loop container, its
maxIterations, its iterator/index variables and its → end out-edge are
untouched, 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 not
taken: a reader who copies the flow does not copy the description.

Measurement — the warning histogram, not the build exit code

flow-loop-body-uncontained is a warning, so pnpm --filter @objectstack/example-showcase build exits 0 on both trees. The reading is the
histogram, anchored on the rule: lines the linter prints:

rule before after
flow-loop-body-uncontained 1 0
flow-try-catch-without-catch 0 0
react-prop-deprecated (control class) 5 5
approval-approvers-may-resolve-empty 10 10
all warnings, all rules 23 22

The 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:

rule: flow-loop-body-uncontained  at flow 'showcase_batch_reminders' ·
  loop 'loop_tasks' body · node 'send_reminder' (notify)

Premise re-verified on origin/main at 3e270d4e2 (the #16069 commit), as the
triage comment asked: the rule still fires on this flow. The two other
try_catch nodes in this file belong to ResilientSyncFlow and
ProjectEscalationFlow, not to this one.

examples/app-showcase/access-matrix.json shows no drift — git status is
clean after both builds. Adding a try_catch inside a flow body does not move
the 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 typechecktsc --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.
  • Gate family derived mechanically on this same head with
    node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands — 32 commands, all run, each exit code captured immediately after
    its own redirect. All 32 green.
    pnpm check:dual-build-cjs-loads first returned exit 3 PREREQUISITE NOT
    MET
    (seven packages had no dist/); those were built and the gate re-run to
    a real green, rather than recorded as a pass.
  • ESLint, narrowed and declared: the repo-wide eslint . --no-inline-config
    scan belongs to CI. Locally the changed file was linted directly — 0 errors,
    0 warnings; --format json reports 1 file linted; --print-config
    resolves 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.mjs never enables type-aware linting for any file
    (no parserOptions.project, no typed @typescript-eslint rules — stated and
    measured 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 in
examples/app-showcase, whose package.json declares "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/** and
packages/lint/** are untouched, and no other flow in the file was changed.

🤖 Generated with Claude Code


Generated by Claude Code

`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
@github-actions github-actions Bot added the size/s label Sep 6, 2026
@os-warren os-warren added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 6, 2026 — with Claude
@os-warren
os-warren marked this pull request as ready for review September 6, 2026 15:18
@os-warren
os-warren enabled auto-merge September 6, 2026 15:19
@os-warren
os-warren added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit c02f70e Sep 6, 2026
38 checks passed
@os-warren
os-warren deleted the claude/issue-14622-batch-reminders-per-iteration-containment branch September 6, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

app-showcase: showcase_batch_reminders loops a notify with no per-iteration containment (first in-repo hit of flow-loop-body-uncontained)

2 participants