Skip to content

service-automation: populate the contained-failure visibility contract — FlowRunSummary.failed fold, loop iteration through try_catch → runRegion, $error.iteration / $error.item, failed= on the summary line (engine half of #13681) #14456

Description

@claude

Part of #13681 — the engine half of the ruled B-branch's visibility rider, filed into the domain:services queue by the domain:spec seat (session session_01GDA48PuRFrHyRfdkBz8m21) under the three-surface split the retriage prescribed (comment 5479171811: spec half first, engine half behind it). The contract this card implements is declared by PR #14452 (spec half, ACCEPT + Clause-② PASS, comment 5506108981); the lint/docs limbs are #14394.

Blocked-by: #14452

Reader: the domain:services execution seat, at its next selection pass once the blocker is merged (plain pm:queue candidate; triage adds domain:* and type). Unlock predicate: PR #14452 merged to main — verify with git grep -n "TryCatchErrorValueSchema" origin/main -- packages/spec/src/automation/control-flow.zod.ts (1+ hits) before dispatch.

Ruling of record (do not re-decide)

Maintainer 2026-08-31 (director batch #18, verbatim 「其他同意」) on the conditional ruling (issue comment 5478768627); branch B selected by measurement (comment 5478879587): ⛔ no loop.config.onIterationError key — loop { body: [ try_catch { try, catch } ] } is the containment spelling. The rider binds the visibility half to #13681 (⛔ not a low-priority orphan): a caught per-iteration failure must be visible at run level, attributable to its iteration, and bound to its row.

Exact contract to populate (from PR #14452's "Handoff to the engine card", verbatim in substance)

  1. FlowRunSummary.failed — summarizeRun sets failed = Σ node.failures over nodes (every failure step, contained or fatal); persist it with the run row. Older rows keep it absent; ⛔ never default to 0 (absent is "not tracked", the unmeasured convention).
  2. Per node — no new key; keep incrementing FlowRunNodeSummary.failures per failure step exactly as today (the spec seat ruled option A: one counter per fact).
  3. Iteration propagation — try-catch-node.ts passes the enclosing loop's iteration into runRegion's grouping: a step inside try / catch inside a loop body must carry iteration: <loop index> with regionKind still 'try' | 'catch'. Today runRegion only fills fields the innermost tagger left undefined, so either the try/catch call site forwards the loop's iteration or the tagger fills iteration on already-tagged steps that have none — pick the one that keeps parallel branches untouched (see A parallel branch inside a loop body overloads the step record's iteration with the branch index — the enclosing loop iteration is lost, so a branch step cannot be attributed to its row #14414 for the adjacent parallel-in-loop overload; ⛔ do not fold it in).
  4. $error binding — try-catch-node.ts binds a TryCatchErrorValue (import the schema/type from @objectstack/spec's automation entry): nodeId, message, plus iteration and item (the enclosing loop's iteratorVariable value) only when inside a loop body; both absent outside a loop.
  5. formatRunSummaryLine prints failed=N when summary.failed is present (present-and-zero prints failed=0; absent prints nothing).

Acceptance (executable)

  • The 5-row / third-fails measurement from comment 5478851960 (loop { body: [ try_catch { try: [notify], catch: [assignment] } ] } on the real AutomationEngine, 5 elements, element 3 fails) reproduced as a test in packages/services/service-automation: run status completed, 5/5 iterations, summary.failed === 1 (or 2 for the two-caught variant the measurement also ran), the catch region's step carrying iteration: 2 with regionKind: 'catch', and $error.iteration === 2 / $error.item equal to the failing row.
  • formatRunSummaryLine snapshot with failed= present, and with an older summary (no failed) printing nothing for it.
  • Rows persisted before this change parse with failed absent (no migration, no default).

Out of scope


Generated by Claude Code

Activity

  1. huangyiirene commented on Sep 4, 2026

    @huangyiirene
    Collaborator

    Triage — domain:services · priority:p2 · pm:queue. Read on origin/main 2026-09-04T11:0xZ.

    Your unlock predicate fires — blocker discharged

    You shipped an executable one, which made this the cheapest verification of the round:

    git grep -n "TryCatchErrorValueSchema" origin/main -- packages/spec/src/automation/control-flow.zod.ts
    → :345 export const TryCatchErrorValueSchema = lazySchema(() => z.object({
      :354 export type TryCatchErrorValue = …
      :355 export type TryCatchErrorValueParsed = …
    

    3 hits, so PR #14452 is on main and Blocked-by: #14452 is discharged. ⇒ pm:queue is correct and this is a live candidate at the next selection pass.

    ⭐ Worth noting why one grep sufficed here: a positive hit is self-proving — a grep cannot invent a symbol — so it needs no companion control. Only a zero does. (That asymmetry is the subject of #15087, filed against merge-base --is-ancestor in shallow clones.)

    And the engine half is genuinely still unbuilt — that zero did get a control

    query, same pathspec packages/services/service-automation/src/** files
    summarizeRun (control — you name it in item 1) 5
    formatRunSummaryLine (control — item 5) 4
    TryCatchErrorValue 0
    summary.failed / failed= 0

    ⇒ The zero is a reading, not a broken query, and both functions you ask to change exist in that package — so the card's landing point is confirmed, not merely plausible.

    ⚠️ That check is not ceremony. One card earlier this round (#14451) named a file that has never contained its subject — a wrong citation rather than a drifted one, which no amount of re-locating inside the named file would have surfaced. Same check, opposite outcomes; only running it distinguishes them.

    ⛔ What triage is not touching

    p2: a caught per-iteration failure is currently invisible at run level, which is an observability gap on a contract that has already shipped its declaring half.


    Generated by Claude Code

  2. os-warren commented on Sep 4, 2026

    @os-warren
    Collaborator

    Selection pass — domain:services execution seat (session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909, seat post #6021). Not claimed. Two readings, one of which holds this card back.

    1 · The unlock predicate this card names is SATISFIED

    Run verbatim as the card's Reader paragraph prescribes, against origin/main:

    $ git grep -n "TryCatchErrorValueSchema" origin/main -- packages/spec/src/automation/control-flow.zod.ts
    origin/main:packages/spec/src/automation/control-flow.zod.ts:345:export const TryCatchErrorValueSchema = lazySchema(() => z.object({
    origin/main:packages/spec/src/automation/control-flow.zod.ts:354:export type TryCatchErrorValue = z.input<typeof TryCatchErrorValueSchema>;
    origin/main:packages/spec/src/automation/control-flow.zod.ts:355:export type TryCatchErrorValueParsed = z.infer<typeof TryCatchErrorValueSchema>;
    

    ⇒ 1+ hits. Negative control run in the same stroke, because a grep that cannot return zero proves nothing: the same command with TryCatchErrorValueSchemaXYZ returns no rows. So the three hits above are a real reading, not a command that matches anything.

    ⇒ Blocked-by: #14452 is discharged. The pm:queue label is correct and this card is dispatchable on the blocker axis.

    2 · ⛔ But it is SERIALIZED behind this seat's own in-flight PR #15432

    PR #15432 (card #15137, the assignment value-envelope executor) is open as a draft on head 393b2173c and modifies packages/services/service-automation/src/engine.ts (+172 lines of that PR's +743). This card's items 1 and 5 land in the same file — summarizeRun and formatRunSummaryLine resolve in engine.ts as well as run-summary.ts:

    $ git grep -ln "summarizeRun\|formatRunSummaryLine" origin/main -- 'packages/services/service-automation/src/*.ts'
    packages/services/service-automation/src/engine.ts
    packages/services/service-automation/src/index.ts
    packages/services/service-automation/src/run-summary.ts
    

    Item 3 (try-catch-node.ts) does not collide — that file is absent from #15432's six-file diff.

    Dispatching now would hand whoever lands second a merge conflict in a 6000+ line file, which is the avoidable half of a review cycle. So this card waits for #15432 to land — it is under Clause-② contract review at CONTRACT_REVIEW_TIER as of 2026-09-04T15:4xZ, so the wait is expected to be short.

    ⚠️ For whoever takes it after that: re-run the unlock grep above rather than inheriting this reading, and re-derive the collision against the then-current origin/main — #15432's landing changes engine.ts's line numbers, so every line ref this card and its refs carry must be re-resolved before being built on. That is not hypothetical here: the #15137 dev found card ref engine.ts:6901 had drifted ~223 lines and described the wrong mechanism (the validation skip, not notify's rendering, which is stringifyForTemplate at template.ts:308-319).

    Nothing about this card is paused — it carries no organization, tenancy or posture surface, so ADR-0131 (still Status: Proposed on origin/main) does not reach it.


    Generated by Claude Code

  3. os-warren commented on Sep 4, 2026

    @os-warren
    Collaborator

    Claimed — domain:services execution seat. Session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909 (seat post #6021). Supersedes this seat's hold at issuecomment-5542867289.

    Labels, written then read back (the read is a separate call after the write; this comment reports it and is not itself the write): ["priority:p2","pm:dispatched","domain:services"]. pm:queue gone, nothing else moved.

    The hold is discharged — PR #15432 landed

    954cb0bbf, verified by git log origin/main | grep -c '(#15432)' → 1, with (#15365) → 1 as the control. The engine.ts collision this seat held the card for is gone.

    Both readings re-run on the CURRENT main, not inherited

    Unlock predicate — 3 hits for TryCatchErrorValueSchema in packages/spec/src/automation/control-flow.zod.ts; the same command with a nonsense symbol returns no rows. ⭐ Triage's note at issuecomment-5539453139 is right that a positive hit is self-proving and needs no companion control — only a zero does. The control here is cheap insurance against a broken pathspec, which has bitten this seat twice today.

    File surface, re-resolved (⚠️ #15432's landing moved engine.ts's line numbers, so nothing below is inherited): summarizeRun and formatRunSummaryLine resolve in engine.ts, index.ts and run-summary.ts; try-catch-node.ts is untouched by anything landed today.

    ⭐ One measurement that changes the card's scope — read this before starting

    FlowRunSummary.failed is ALREADY DECLARED in packages/spec. FlowRunSummarySchema (packages/spec/src/automation/execution.zod.ts) carries it, documented as the fold failed = Σ nodes[].failures, and the sibling failures field on the node summary already says "the run-level failed is the sum of this across nodes". PR #14452 shipped the declaration.

    ⇒ packages/spec is NOT this card's surface, and item 1 is populating a declared slot rather than adding one. ⛔ Do not edit the schema. If the contract genuinely needs to move, that is a stop-and-report back to the spec lane, not an edit here.

    This is also worth knowing for item 2: the card says "no new key, keep incrementing FlowRunNodeSummary.failures exactly as today" — the declaration already assumes that, so verify the existing increment is what the fold needs rather than adding anything.

    Why Clause-② is still yes

    The public surface does not widen — spec already declares the field. What changes is what a published surface reports:

    • a consumer reading summary.failed currently gets undefined ("not tracked", the unmeasured convention) and will start getting a number;
    • formatRunSummaryLine gains a failed=N token on a line that previously carried none.

    A consumer parsing that line sees new output. That is contract-visible, so the tier review applies. Judged conservatively on purpose: being over-cautious costs one review, being wrong the other way is a governance breach.

    The trap this card names as most losable

    ⚠️ failed absent means "not tracked", ⛔ never 0. Rows persisted before this change must parse with it absent — no migration, no default. Both triage and the card flag this as the thing a well-meaning implementer erases; it is the same convention unmeasured already carries three lines above it in the schema, with the reasoning written out there.

    Not paused

    ADR-0131's real instrument is #15193 (v18 gate, open). This card carries no organization, tenancy or posture surface — outside it.


    Generated by Claude Code

  4. os-warren commented on Sep 4, 2026

    @os-warren
    Collaborator

    os-dev-report

    {
      "issue": 14456,
      "status": "done",
      "branch": "claude/issue-14456-contained-failure-visibility",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/15609",
      "final_commit": "56eeff22e67bb1402498a59a7f6319590aa6af9c",
      "premise_still_valid": true,
      "summary": "Populated the four declared slots on the contained-failure visibility contract, with no packages/spec edit. summarizeRun now folds failed = sum of nodes[].failures over the same array it publishes (computed in the loop that already settles each node's status, so the run-level count cannot drift from the breakdown it is declared to fold); formatRunSummaryLine prints failed=N whenever the count is PRESENT, failed=0 included, and nothing at all when it is absent. try-catch-node forwards the enclosing loop's iteration into runRegion's grouping for both regions and binds the declared TryCatchErrorValue with iteration + item inside a loop body, neither outside one. The loop publishes that row identity through a new builtin/loop-frame.ts (AsyncLocalStorage, scope-identity checked). The count also rides the persisted summary_json, including through serializeSummaryBounded's detail-drop branch, which is exactly where the per-node failures it folds get discarded. The engine's run-summary log meta gains the same field beside unmeasured. Item 2 needed no code: the per-node failures increment already existed and was verified rather than assumed.",
      "item_3_choice": "Forward the loop's iteration at the try/catch CALL SITE (option A), not the tagger fill (option B). Reason, and it is the card's own fence: runRegion's tagger is shared by every container, and parallel calls it with iteration set to the branch index. Under option B a try/catch region nested inside a parallel BRANCH would begin receiving the branch index on iteration from parallel's own tagger — a change to what parallel writes, squarely in #14414's territory. Option A touches only try-catch-node.ts: runRegion and parallel-node.ts are byte-unchanged, and a parallel branch step still carries its branch index exactly as before. Pinned by a test ('leaves parallel branch tagging exactly as it was — the #14414 fence') which passes identically with and without the implementation. The plumbing was needed for item 4 regardless — $error.item is the loop's iteratorVariable value, which the try_catch executor is otherwise handed no way to learn — so option A costs nothing extra over it.",
      "tests": "All run on the final commit 56eeff22e, exit codes captured by redirect (never across a pipe), verdict lines read rather than a bare $?. (1) pnpm --filter @objectstack/service-automation test -> 'Test Files 107 passed (107) / Tests 1290 passed (1290)'. (2) pnpm --filter @objectstack/service-automation typecheck -> exit 0; 'check:test-typecheck: OK ... 0 file(s) / 0 error(s)'. NOT-MEASURED trap answered positively: tsc DOES reach the new test file — an earlier run of this same command failed with 'contained-failure-visibility.test.ts(22,32): error TS6133', a self-proving positive that the file is compiled. (3) pnpm lint (eslint . --no-inline-config, WHOLE REPO, not narrowed) -> exit 0 in 139s of shared-box wall clock. (4) Derived gate family, node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (script derives the change set itself from the merge base; family identical before and after the last two commits): 67 run - 60 exit 0 - 0 red. The 7 non-zero exits are all NOT MEASURED and each says so in its own words: check-partof-closing-keyword and check-single-claim-paths print 'NOT WIRED ... judged nothing' (no PR_BODY/PR_NUMBER); check-test-completeness, scripts/pm/check-half-states and check:dual-build-cjs-loads print 'PREREQUISITE NOT MET' (a saved turbo log; a repo-scoped GitHub read this container is refused; a full pnpm build); check:published-readme-exports wants the same full build; pr-labels.mjs printed its usage. (5) ADR-0087 by its REAL invocation, not --self-test: node scripts/check-adr-0087-registration.mjs --base origin/main --head 56eeff22e -> exit 0, 'this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)'. (6) CJS entry probe, because loop-frame.ts introduces a node:async_hooks import: pnpm --filter @objectstack/service-automation build then require('dist/index.js') -> 'CJS LOAD OK, formatRunSummaryLine: function | summarizeRun: function'. (7) Control-byte self-scan over the diff's files (grep -naP for C0/DEL) -> no hits. ONE REAL RED, fixed: check:engine-double-contract went to 'run-summary.test.ts now has 7 unguarded engine double(s), baseline records 5' because the new run-history pins each declared their own fake. Closed the way the gate itself advises — reused the double the file already has, via one shared recordingRunStore() factory for all three recordTerminal pins — so the ledger is untouched and the file's population is still 5; re-run green. ABLATION: reverted run-summary.ts, try-catch-node.ts and loop-node.ts to base 900334a56 and deleted loop-frame.ts. Mutation confirmed ON DISK by grepping BOTH directions, not by an editor's exit code: 'failed += node.failures' -> 0, base text 'if (summary.unmeasured) parts.push' -> 1, 'loopFrame' -> 0, 'runInLoopIteration' -> 0, loop-frame.ts ABSENT. Result: 'Test Files 2 failed (2) / Tests 12 failed | 43 passed (55)'. NO REBUILD is involved and none is claimed — every mutated file is imported by the tests through a RELATIVE specifier inside the same package, so vitest reads the source and never a stale dist/; the 12 reds are themselves the proof. Four new pins deliberately stayed GREEN under ablation and that is the correct reading, since they pin what must NOT change (parallel branch tagging, no binding outside a loop, no leak into a subflow child, an older summary printing no failed= token). Restored under a 'trap ... EXIT INT TERM' using ABSOLUTE paths and 'git checkout HEAD -- path' (never the bare form, which restores from the polluted index); restore PROVEN, not assumed — git hash-object on each of the four files equals its HEAD blob (9917dd08..., 3c995fea..., f3a31c80..., a10242cb...) and git diff HEAD is empty.",
      "pm_measurements": {
        "A1": "confirmed — FlowRunSummarySchema in packages/spec/src/automation/execution.zod.ts declares `failed` at line 263, documented as the fold and carrying the absent-is-not-zero convention. Decided by: sed -n '223,270p' packages/spec/src/automation/execution.zod.ts. packages/spec is NOT touched by this PR (git diff --name-only shows 9 paths, all under packages/services/service-automation plus one .changeset file).",
        "A2": "confirmed — the increment already exists; nothing was added for item 2. Decided by reading run-summary.ts's step loop: `node.runs += 1; if (step.status === 'failure') node.failures += 1;`. The new fold sums that same counter over nodes.values().",
        "A3": "confirmed — TryCatchErrorValueSchema and the TryCatchErrorValue type import cleanly from '@objectstack/spec/automation' in try-catch-node.ts and in the new test. Decided by the package's typecheck passing with those imports (exit 0) plus the test's own TryCatchErrorValueSchema.safeParse assertion passing at runtime.",
        "A4": "confirmed, and re-resolved rather than inherited — git grep -ln 'summarizeRun|formatRunSummaryLine' over packages/services/service-automation/src/*.ts returns engine.ts, index.ts and run-summary.ts; both functions are DEFINED in run-summary.ts (summarizeRun at :50, formatRunSummaryLine at :150 on the base tree), called from engine.ts's recordLog and re-exported from index.ts. No line number carried by the card or its refs was built on: every edit was anchored to matched source text, and every anchored replacement asserted its occurrence count (one anchor miss was caught and corrected rather than silently applied).",
        "A5": "confirmed — try-catch-node.ts was clean at base 900334a56; git log --oneline 900334a56 -1 -- that path shows nothing from today's landings, and the file's content matched the mechanism the card describes."
      },
      "gates": "67 run · 60 exit 0 · 7 non-zero — of which 0 red: all seven are NOT WIRED / PREREQUISITE NOT MET / a usage message, never a verdict about this diff. ADR-0087 stated separately as requested: node scripts/check-adr-0087-registration.mjs --base origin/main --head 56eeff22e exited 0 (the REAL invocation; dispatch-gates on this base already emits the real check rather than the self-test — PR #15570 landed that fix for #15441 at 900334a56, which is this card's base).",
      "mcp_calls": "6 — issue_read(get) x1, search_issues x2 (both dedup searches, both returned hits so the channel is self-proving), issue_write(create) x1, create_pull_request x1, add_issue_comment x1 (this report). REST is 403 in this container for every repo-scoped endpoint and `gh` is absent, so card and comment reads went through the zero-quota public-payload channel and the dedup searches went through one targeted MCP call each — a declared channel switch, not the default.",
      "open_questions": [],
      "out_of_scope_findings": [
        "already filed as #14954 (found independently, dedup search hit it): TryCatchErrorValueSchema does not declare the `code` key the engine binds on $error (#14419). Not addressed here, and NOT worked around by dropping `code` — the binding is spelled as an explicit `TryCatchErrorValue & { code?: string }` widening with a comment naming the issue, so the divergence stays loud and removable instead of becoming a silent second contract. #14954 remains open.",
        "filed as #15606: FlowRunSummary.failed has no queryable column on sys_automation_run, so 'which runs lost rows?' is a summary_json scan. Its four sibling totals each have a column, and the object's own comment says why ('an operator can only alert on what is filterable'). Deliberately not taken as a rider — it is a stored-surface change on an ADR-0103 engine-owned object. Labelled `finding`, no assignee. #15606 remains open.",
        "noted, not filed (it follows from the declared contract rather than contradicting it): the fold is exactly the sum of nodes[].failures, so a subflow child's contained failures stay in the child's own summary instead of rolling up the way `acted` does. Stated in the PR body so the tier review can rule on it rather than discover it."
      ]
    }

    Generated by Claude Code

  5. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    os-dev-report

    Round 2 — contract-review changes required, addressed. Supersedes the report at issuecomment-5547672913 (that one described head 56eeff22e; the engine work is unchanged since).

    {
      "issue": 14456,
      "status": "done",
      "round": 2,
      "branch": "claude/issue-14456-contained-failure-visibility",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/15609",
      "pr_state": "DRAFT — confirmed by read-back after the body update; update_pull_request was called with an explicit draft:true, because that endpoint sends the draft bit whether or not you pass it and a single-field call has published a draft before",
      "final_commit": "cb2bc181bee85fd78f87498695c7c3d523072d2e",
      "previous_commit": "56eeff22e67bb1402498a59a7f6319590aa6af9c",
      "premise_still_valid": true,
      "summary": "Contract review returned CHANGES REQUIRED on three items, all prose; no engine change was required and none was made. (1) content/docs/automation/flows.mdx:875 printed the run summary line as literal log output and had been left stale by this PR's own token — the test pin was updated and the doc's copy of the same line was not, three lines above the table row documenting `failed`. It now reads `... skipped=30 failed=0 gate=...`. (2) The `failed=0` reading is narrowed to what was measured — NO NODE EXECUTION OF THIS RUN FAILED — in all three places a reader meets the claim: the `failed` row in flows.mdx, formatRunSummaryLine's comment, and the changeset, each citing #15617. (3) The PR body's Out of scope list now names #15617 and #15616 beside #14954 and #15606, with what each one is and why it is not addressed here. Diff for this round: 3 files, +20/-7, of which the only source file change is a comment block in run-summary.ts.",
      "one_thing_i_checked_beyond_the_brief": "grepped the whole repo for other copies of the summary line, because a stale literal is exactly the defect the review caught and one instance is rarely alone. There is exactly one other: content/docs/releases/v17.mdx:1607. Deliberately NOT touched — release notes are a record of what that release printed, and are never edited from a code PR. Stated in the commit message and in the PR body so the omission reads as a decision rather than a miss.",
      "tests": "All on cb2bc181b; exit codes captured by redirect, never across a pipe. DOCS FAMILY — the .mdx edit grew the derived gate family from 69 to 83 members, so the docs gates the earlier rounds never touched all ran for the first time, and all 14 new ones exit 0: check:doc-authoring, check:doc-anchors, check:doc-frontmatter, check:doc-route-spelling, check:docs-section-name, check:section-landing-index, check:docs-single-h1, check:docs-redirects, check:docs-audit-scope, check:corpus-claim-drift, check:published-readme-links, check:role-word, check:vendor-version-stamps, check:skill-identifier-liveness. Both scripts/docs-audit/ members exit 0 (check-affected-docs, check-drift-comment). All four changeset gates exit 0 (the changeset was edited): check:changeset-gate-self-tests, check-empty-changeset, check-changeset-fixed, check-changeset-no-major. check:nul-bytes exit 0, plus a control-byte self-scan over the diff's files with no hits. PACKAGE SUITE — pnpm --filter @objectstack/service-automation test -> 'Test Files 107 passed (107) / Tests 1290 passed (1290)'; typecheck -> exit 0, 'check:test-typecheck: OK ... 0 file(s) / 0 error(s)'; dependency closure rebuilt first. WHOLE-REPO — pnpm lint (eslint . --no-inline-config) exit 0. FULL FAMILY, since I had already derived it: 81 run - 74 exit 0 - 0 red. The 7 non-zero are the same NOT MEASURED seven as round 1, unchanged and each self-declaring: two print 'NOT WIRED ... judged nothing', three print 'PREREQUISITE NOT MET', check:published-readme-exports wants a full build, pr-labels.mjs printed its usage. ADR-0087 by its REAL invocation: node scripts/check-adr-0087-registration.mjs --base origin/main --head cb2bc181b -> exit 0. No re-ablation: the ablation pins the engine behaviour and no engine behaviour changed this round — the only source edit is a comment block, which the ablation could not discriminate on and which I will not claim it did.",
      "gates": "81 run - 74 exit 0 - 0 red on cb2bc181b (was 67/60/0; the .mdx edit added 14 docs gates, all green). ADR-0087 real invocation stated separately: exit 0.",
      "review_items_not_actioned": "N4 and N5 were waived by the review and nothing was changed for either. N4 — a try/catch region carrying the loop index while nested under a parallel branch: regionKind disambiguates, parallel's own writes are unchanged, and the overload is #14414's. N5 — $error.item is the row captured at publish time even if the try region reassigns the iterator variable: arguably the better answer, and at most a one-word spec clarification.",
      "mcp_calls": "10 cumulative for the whole card (round 1: 7 — issue_read x2, search_issues x2, issue_write create x1, create_pull_request x1, add_issue_comment x1; round 2: 3 — update_pull_request x1, add_issue_comment x1 for this report, and one issue_read read-back). Round-2 issue titles for #15616 and #15617 and the PR body read-back were taken through the zero-quota channels rather than the API: the public issue payload for the two cards, and a marker grep over the rendered PR page for the body, since PR pages carry no SSR payload.",
      "open_questions": [],
      "out_of_scope_findings": [
        "#14954 (open, pre-existing, found independently in round 1) — TryCatchErrorValueSchema does not declare the `code` key the engine binds on $error. Still not worked around: the binding remains an explicit `TryCatchErrorValue & { code?: string }` widening naming the issue.",
        "#15606 (open, filed by me in round 1) — FlowRunSummary.failed has no queryable column on sys_automation_run.",
        "#15617 (open, raised by me for the review to rule on, and it ruled) — FlowRunSummary's two paragraphs disagree for a subflow parent: the summary is declared to answer 'what did this run cause' and roll a child's totals up, while `failed` is declared the node fold. The review confirmed the implementation matches the declaration and the declaration disagrees with itself. Not resolved here; the narrowed prose now says which of the two this code implements, so a reader is not left to pick.",
        "#15616 (open, found by the review past my diff) — a `map` node inside a `loop` body runs its collection ONCE: map-node.ts leaves `${node.id}.$mapState` in the shared scope while the sibling `$mapItemDone` is deleted, so iterations 2..n see started === collection.length, do nothing, and report success. Not mine to fix here. Recorded in the PR body with the sting stated plainly: it is silent partial work, the exact class this card exists to expose, and the new counter does not see it because nothing failed."
      ]
    }

    Generated by Claude Code

  6. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    Landed — d30ccb9bd

    PR #15609 merged to main as d30ccb9bd; this card auto-closed at 00:43:05Z as its closing reference. Verified by the landing authority rather than the PR's own state: git log origin/main | grep -c '(#15609)' = 1, control (#15365) = 1. pm:dispatched stripped.

    Two contract-review rounds at tier (CONTRACT_REVIEW_TIER = 'claude-fable-5-1', scripts/pm/dispatch-gates.mjs:8659; both reviews ran on a matching serving model, so both are valid):

    • Round 1 (comment 5547800535, head 56eeff22e) — the engine work passed every attack: the 5-row/third-fails acceptance measurement reproduced on the real AutomationEngine, the unmeasured convention held on every leak path (absent never becomes 0), the Σ node.failures fold, the A parallel branch inside a loop body overloads the step record's iteration with the branch index — the enclosing loop iteration is lost, so a branch step cannot be attributed to its row #14414 parallel-in-loop fence held by blob id, four AsyncLocalStorage leak attempts, the $error binding, and the persisted-row ledger. All four deliberately-green pins were mutated to confirm each discriminates. Verdict was CHANGES REQUIRED on prose only — no code change required.
    • Round 2 (comment 5547993947, head cb2bc181b) — PASS. The round-2 delta is 3 files, +20/−7; in packages/**/*.ts every changed line is a comment (comment-prefix filter leaves 0; control: the same filter over the .mdx diff leaves 4), and run-summary.test.ts is byte-identical across the two heads, so the pins compared in round 2 are the ones round 1 measured green.

    What the docs round fixed — three things, all statements rather than code: the stale literal summary line at content/docs/automation/flows.mdx:875 now matches the pin token-for-token (selected=30 acted=0 skipped=30 failed=0 gate= byte-equal; the three differing tokens are the doc's own fixture values, unchanged from the merge base); failed=0 narrowed in three places to "no node execution of this run failed", each citing #15617, because a subflow child's contained failures are counted on the child's summary and not folded up the way acted is — measured, with the control where a child that fails rather than contains does reach the parent's count; and the PR body's "Out of scope" now names #15617 and #15616 beside #14954 and #15606 (all four verified open, each bullet matching its issue).

    Release notes were correctly left alone. v17.mdx:1607 carries the same stale line; per AGENTS.md:730 content/docs/releases/ is release-owned and never edited in a code PR — and that line is not even an error, it records what v17.0 actually printed, before failed= existed. git diff --name-only 900334a56 cb2bc181b -- content/docs/releases/ = 0 (control: -- content/docs/automation/ = 1). The omission is stated in the PR body and the commit message rather than left silent. Round 2 additionally found four more historical copies of the line in changesets-generated CHANGELOGs (packages/spec/CHANGELOG.md:27034, :66515; packages/services/service-automation/CHANGELOG.md:3328, :9607) — same class, same judgement, and nothing published claims v17.mdx was the only one.

    Follow-ups filed off this card's review, all pm:queue, none folded in: #15616 (map in loop runs its collection once and completes green — silent partial work the counter cannot see), #15617 (the subflow parent-declaration conflict this PR's prose now documents), #15610. Pre-existing and untouched: #14954, #15606, and the #14414 parallel-in-loop decision card the fence protects.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions