fix(lint): flow-decision-unconditional-branch reports the decision that gates on nothing

[claude]

Fixes #16093

lintFlowPatterns reported flow-decision-unconditional-branch only when a decision had both gated and ungated out-edges. One line decided that:

const gated = outs.filter((e) => e.condition || e.isDefault === true);
if (gated.length === 0) continue; // no branching declared at all — nothing to undercut

So the strictly worse shape — a decision whose out-edges carry no condition and no isDefault, and whose node declares no config.conditions[] — fell out of the loop before any finding was pushed. The comment was the defect's own confession: the rule was framed as "an unconditional edge undercuts a guarded one", and under that framing zero guarded edges honestly read as nothing to undercut. The framing was wrong, not the implementation. A decision with no guard anywhere does not have less wrong with it — it selects no branch at all, every successor runs on every pass, and the gateway is decoration. It is also the harder shape to notice in review, because the node still says type: 'decision'.

The runtime confirms the boundary, so the linter uses the engine's own predicate

The decision executor in packages/services/service-automation/src/builtin/logic-nodes.ts divides on exactly one condition, and says so in its own docblock:

• it declared config.conditions — the first matching entry's label is the branch, and traversal restricts itself to the out-edge carrying that label;
• it declared none — "it is a plain gateway: the branching lives on the out-edges (condition / isDefault) and the node reports no branch."

execute() implements that as if (conditions.length === 0) return { success: true };. The new lint branch tests Array.isArray(cfg.conditions) && cfg.conditions.length > 0 — the same boundary the executor draws, rather than a second hand-rolled reading of it. That is also why a bare edge label is not treated as a guard here: for a decision, a label selects a path only when the node reports a branchLabel, which requires conditions[]. The mixed branch of this same rule has always counted a labelled edge the decision cannot select as ungated, so both halves of one rule id now answer the same way.

What changed

Same rule id, same warning tier, with its own message. The mixed shape keeps its existing wording and its single finding; the fully-inert shape gets a sentence that names the out-edges running unconditionally and offers the three real fixes (a condition per branch plus isDefault: true on the fallback, a config.conditions[] whose label matches an out-edge, or dropping type: 'decision' for the node the gateway already behaves as).

A new rule id was considered and deliberately not taken — see the note at the end.

Two shapes stay silent, on purpose, and both are pinned:

• an ordinary gateway (guarded edges plus a matching conditions[]) — the control the triage named as the easiest thing to break;
• a decision routing by config.conditions[] alone with bare out-edges — its branching is declared on the node, so it is not inert.

And a decision declaring a label no out-edge claims stays the gating flow-branch-label-unmatched alone: the inert branch excludes it rather than piling a second, contradictory finding on the same node.

Premise check, before the first edit

The card's measurement was taken on the pinned @objectstack/lint@17.3.0 as installed by hotcrm, not on this tree, so the gap was re-established here first with the package's own harness. Two readings, one of which corrected the dispatch's expectation:

• The guard was where the card said (packages/lint/src/lint-flow-patterns.ts, the gated.length === 0 short-circuit), under rule: FLOW_DECISION_UNCONDITIONAL_BRANCH. Confirmed.
• The dispatch expected no existing test to cover the fully-inert shape. That was falsified, in the strongest possible direction: lint-flow-patterns.test.ts carried it('does NOT flag a decision with no guarded edge at all — nothing to undercut'), asserting toHaveLength(0) on precisely the shape this card is about. The defect was pinned. That fixture pins the branch being removed, so it was replaced outright rather than edited — it is now the positive case.

Red-first on this tree, against the unmodified rule: the four new positive cases failed with expected [] to have a length of 1 but got +0 — zero findings on the fully-inert decision — while all 135 other cases, including every negative control, passed.

One existing negative control had to be re-scoped, and why that is not a weakening

flow-error-label-not-fault's case does NOT flag label:'error' out of a decision node asserted a global zero over all findings. That global zero held only because the fully-inert decision was invisible: its decision fixture declares no conditions[] and guards no edge, so it now correctly draws a flow-decision-unconditional-branch warning of its own. Exempting labelled edges to preserve the zero would make one rule id answer two ways, so instead the assertion was scoped to the rule it is about. What the case tests is unchanged: an error-ish label out of a branching node is not the error-routing footgun. The reasoning is recorded at the assertion.

Verification

Head sha 80202cc59.

• pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/lint-flow-patterns.test.ts — 139 passed, exit 0.
• pnpm --filter @objectstack/lint test through the shared lock — VERDICT command-exit 0, 100 files / 3407 passed, 5 skipped.
• pnpm --filter @objectstack/lint typecheck through the lock — VERDICT command-exit 0 (including the test-layer debt check).
• Consumers. turbo ls --affected names 54 packages, which is the transitive closure below a leaf dependency rather than the set that can observe this change. The only call site of the changed function in the repo is AUTHORING_RULES in packages/lint/src/authoring-rules.ts, reached by os validate / build / lint and the runtime publish gate, and the new finding is warning-tier so it fails neither. Every direct dependent of @objectstack/lint was run instead: metadata-protocol, objectql, cloud-connection, mcp, example-showcase, platform-objects (all via turbo test, 88 tasks, all VERDICT command-exit 0) and @objectstack/cli (--project unit, 181 files / 2453 passed, 6 expected-fail). The CLI's integration tier is declared to CI — this diff touches no integration-layer file and no spawn entrypoint.
• Gates. All 54 commands node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derives for this diff were run at the head sha: 54 exit 0. dispatch-gates --ran reconciles: 54 derived, 54 run, 0 NOT-MEASURED, 0 UNRUN. That set includes check:docs-transcript-drift (green — the signal that no new rule id crept in), check:nul-bytes, check:type-check-coverage, check:type-check-debt, check:test-source-alias and check:cross-package-test-inputs.
• check:doc-authoring caught a real defect in the first draft and is worth naming: the new hint ended with a bare tracker id, and a runtime string reaches authors and operators, who cannot resolve one. The id was stripped from the string, not added to the baseline; the provenance stays in the adjacent source comment.
• Lint. eslint . --no-inline-config --format json was run over the whole repo rather than narrowed: 6226 files, 0 errors, 0 warnings, exit 0.
• Control-character sweep over the three changed files: clean.

Ablation

The continue was restored on top of the committed fix and the suite re-run. The mutation was proven on disk before reading any result — the new message text went 2 occurrences to 0, the original short-circuit line 0 to 1, and the blob hash differed from HEAD's. Direction: red, exactly as predicted — the same 4 new positive cases failed and 135 passed, so every negative control stayed green under ablation. Restore was proven by blob hash equal to HEAD plus an empty git diff HEAD, not by an exit code. The first ablation attempt aborted on its own guard: the guard's expected occurrence count for a prose phrase was mis-stated (the phrase appears twice in comments, not once). The mutation itself had landed correctly; only the anchor was corrected, and the run was not repeated until something happened to land.

Scope notes

• A distinct rule id was considered and not taken. The triage's reading is that a decision with one unconditional out-edge and no conditions[] is indistinguishable from a noop node, so a distinct message may read better than folding it into the mixed-branch wording. That is exactly what this PR delivers — a distinct message under the existing id. A distinct id would add an exported constant, a registry entry and a rule-count move across the docs transcripts, which is out of this card's dispatched scope. Nothing found while implementing argues the id must split: both shapes are one defect class (a decision whose declared routing does not route), they share a severity tier, and a test pins that they do not double-report.
• Card 15429 is the multi-guarded-edge case (nothing enforces that intended-exclusive conditional edges partition). This card is the zero-guarded-edge case. They do not overlap and this change does not constrain that one: everything here fires only when gated.length === 0, which is the complement of the situation card 15429 is about. The two also agree on the conditions[] versus edge-condition relationship — one mechanism per decision, never both — which is the shared advice the triage asked to keep consistent.
• Card 4414 is the family origin; the survey that found this is hotcrm card 1613.
• The branch is 7 commits behind origin/main at report time. None of those commits touch either changed file; dispatch-gates flags the staleness only for the closing-keyword guard family, which is not in this diff's derived set.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0374bcba9f7108a4fec6624e9f4c39f712d618c7 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9b18ea98ffa4f96c5d549f2d3003c940883fa699 — the merge of head 80202cc59d22c425a1dda8f3d47044b6a60b9c78 into base 0374bcba9f7108a4fec6624e9f4c39f712d618c7, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9b18ea98ffa4f96c5d549f2d3003c940883fa699 && git checkout 9b18ea98ffa4f96c5d549f2d3003c940883fa699
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0374bcba9f7108a4fec6624e9f4c39f712d618c7 80202cc59d22c425a1dda8f3d47044b6a60b9c78 && git checkout -B drift-repro 0374bcba9f7108a4fec6624e9f4c39f712d618c7 && git merge --no-ff 80202cc59d22c425a1dda8f3d47044b6a60b9c78

node scripts/docs-audit/affected-docs.mjs --json 0374bcba9f7108a4fec6624e9f4c39f712d618c7

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

[claude]

Standing-down note — shard 5 timed out twice; the aggregate read red honestly; the queue regrouped a third time — PM seat domain:devx @ objectstack (#6023), session session_01Vbw3RPgdtqesx4azk9SbW8, 18:05Z.

This PR (head 80202cc59, ACCEPT 5560732762) entered the queue 17:22:00Z. Both group runs lost Test Core (5/6) to the 30-minute job wall and, since PR #16316, the required Test Core aggregate reads that as failure rather than passing over the untested shard:

group run	Test Core (5/6)	Test Core (6/6)	aggregate
34048392667 (17:22Z)	cancelled at 30m09s	success 27m58s	failure 17:53Z
34048918210 (17:32Z, regroup)	cancelled at 30m16s	success 27m42s	failure 18:03Z

Not this PR's failure: the diff is packages/lint/src/lint-flow-patterns.ts + its test + a changeset; every PR-side row at this head was green (each shard incl. 5/6 at 22 min), and shard 5 is the CLI slice whose predicted time is stale (#16173, pm:awaiting-maintainer for the dataset refresh — the only lever). Recorded on #16173 as the first honest queue red (5561098688).

Correction at 18:07Z, before any action was taken: the queue did NOT remove this PR — it regrouped it a third time (group run 34050511342, created 18:03:36Z, shard 5 running again; a regroup happens when a PR ahead lands and the group base moves). So the PR is still queued and nothing is re-armed; auto_merge: false on a queued PR is the known API shape, not a signal. The seat's one re-queue stays unspent: if the queue removes this PR after the third run, re-queue once via auto-merge; a removal after that parks this PR as ACCEPTED-awaiting-queue-capacity and reports that #16173's refresh gates it. ⛔ No test is skipped, no shard is retried by hand, no empty commit.

Queue state at 18:05Z: the timeline had not yet shown removed_from_merge_queue (events lag); re-arm reads the entry event, never the auto_merge field.

---

Generated by Claude Code