Skip to content

fix(service-automation): one renderer for the contested-flow phrase, and the two spellings it had drifted into - #12607

Merged
os-litant merged 1 commit into
mainfrom
claude/issue-12563-flow-shadowing-renderer
Aug 26, 2026
Merged

fix(service-automation): one renderer for the contested-flow phrase, and the two spellings it had drifted into#12607
os-litant merged 1 commit into
mainfrom
claude/issue-12563-flow-shadowing-renderer

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #12563

One event — a flow name claimed by more than one definition — was described to an operator in three places, each with its own private const describe beside the log call. Nothing held the copies equal, and two axes had already drifted before anyone looked.

The two copies inside @objectstack/service-automation are now one exported renderer, renderFlowContender. The third copy, in @objectstack/cli's startup banner, deliberately keeps its own rendering and is held equal by a test-only agreement pin. Why that split is the shape, measured rather than assumed, is the first section below.

The boundary question, answered before writing code

The card's shape A ("export a renderer, have all three call it") required knowing what a value import would cost packages/cli. Measured on origin/main:

measurement reading
static value imports of @objectstack/service-automation in packages/cli/src 0
dynamic value imports 1await import(...) in utils/data-migration-plugins.ts, behind opts.automation === true
imports of that package in utils/format.ts 0 — not even import type; only a prose mention
modules in packages/cli/src importing utils/format.js 56

So a renderer import into format.ts would be this package's first static value import of the automation package, placed in a util that every CLI command's module graph already reaches — os whoami, os init, os login included. That is precisely the property PR #12562 spent effort buying: its engine reads are structural and feature-detected so a host on an older automation package still boots its banner.

A-full is not available. The fold ends the class where the producer lives; the boundary is held by a pin instead.

Both divergence axes, decided on measurement

1. Quoting — measured against the producing package, never majority-voted.

scope single-quoted double-quoted
packages/services/service-automation/src 203 3
packages/objectql/src 170 37
packages/cli/src 97 73

One of the producing package's 3 double-quoted uses was this phrase. The sentence it lands in already single-quotes the flow name, which is the more free-form of the two values, so single quotes add no ambiguity the line does not already carry. packageId is an unconstrained z.string() in packages/spec (no character pattern anywhere), so neither spelling is provably safe against an adversarial id — this one is at least the house convention. ⇒ single.

2. Absent packageId. The two engine copies interpolated a bare undefined; the CLI copy rendered a real fallback. The renderer takes the CLI's answer, a code-shipped package (id unknown).

This package's own callers cannot reach that branch today — isCodeArtifactBody is false on a falsy _packageId, so a source: 'package' contender always carries one. That is recorded on the card as a non-premise, not a live bug, and it is not treated as one here. But unreachability is a property of today's callers, not of an exported function, so the renderer is made safe rather than left safe-by-luck.

The trap the card is actually about

describeFlowContender is exported, sits exactly where a shared renderer would live, and reads like one — but it is a classifier with no prose in it. A missing renderer is a gap someone fills; a misleading one is a gap that keeps getting re-filled privately, which is what the third copy was. Its TSDoc now says what it is not, and points at the renderer.

What holds the copies equal, proven by ablation

Two layers, deliberately non-redundant — the call sites assert through the renderer, and the renderer's own output is pinned as literals, so the two cannot drift together and stay green.

ablation (mutation proven on disk before any verdict was read) result
A — renderer reverts to double quotes, rebuilt into dist/ engine 1 failed / 13 passed; cli pin 2 failed / 2 passed
B — a fourth caller re-invents the phrase privately at the warning site, renderer untouched engine 1 failed / 13 passed — the derived row only; the three literal pins stayed green
C — the CLI side drifts instead, engine untouched cli pin 2 failed / 2 passed

Every leg: unique-anchor mutation refusing to run on a non-unique or zero-hit anchor, post-write removed/injected counts observed, blob hash compared against the HEAD blob, ablation-dist-preflight for presence on the mutation leg and --absent on the restore leg, and restoration proven by an empty git diff HEAD plus a matching hash. Ablation C mutated packages/cli/src/utils/format.ts transiently in the local worktree only — that file is not edited by this PR and was restored byte-identically (eb214450f7e96edd75a67e65ee06022ca3aa4161 before and after).

A first attempt at ablation A was a no-op — the perl substitution matched zero times and exited 0. It was caught by the byte-hash check before any verdict was read, and is recorded here because "editor exit code" and "the edit happened" are different facts.

Cross-lane fence

packages/cli/src/utils/format.ts belongs to the domain:cli lane and was not cleared at dispatch. Cleared here, immediately before the work: no open claude/ PR touches that file or packages/services/service-automation/ (all 9 open branches fetched and diffed against origin/main), and same-day churn on it is only PR #12562, already merged. This PR does not edit that file in any case.

Verification

Suites, on ba3a391:

  • @objectstack/service-automation full suite — 91 files, 1086 tests, all passed. flow-name-shadowing.test.ts went 11 to 14 cases: all 11 original names survive, 3 added. No case moved — the only change to a pre-existing case is the expected string in the precedence-warning row, which is prose, not classification.
  • @objectstack/cli — the 6 banner/format suites, 50 tests, all passed, including the new pin's 4.
  • tsc --noEmit on packages/cli: clean, and --listFiles confirms the new test file is in the program (a green typecheck that never read it would have said nothing).
  • packages/services/service-automation has no typecheck script (it is a DEBT-ledger package), so a --filter run of typecheck matches zero scripts and exits 0 having measured nothing. Measured directly instead: tsc --noEmit reports exactly 3 errors, all pre-existing TS2341 in nested-region-parity.test.ts, matching the ledger's recorded count. Debt unchanged.

Gates — union re-derived from the actual changed set via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack on this tree, exit codes captured before any pipe. 25 families, all green. Three first returned PREREQUISITE NOT MET (NOT MEASURED, not red): check:i18n and check:i18n-coverage needed the built CLI, and check:type-check-debt refused to re-measure without the workspace closure. Both readings are recorded — after turbo run build over packages, all three are OK, with check-type-check-coverage --re-measure: OK — 31 ledger entries re-measured, none above its recorded number.

Worth naming: check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through dist. The new cross-package import does not widen the shrink-only KNOWN_UNALIASED_TEST_IMPORTS ledger — @objectstack/service-automation was already registered for @objectstack/cli via the dynamic import named above, so the set is unchanged. The alternative, aliasing that dep to source in vitest.config.ts, was deliberately refused: it would pull that package's entire import surface into this package's resolution domain for all ~185 test files, to buy staleness-resistance on one three-branch pure function.

pnpm lint (repo-scale, eslint . --no-inline-config) — exit 0, 55s, run in full rather than narrowed.

Changeset

minor on @objectstack/service-automation, argued in the file: it adds an export to a published package's public API and changes shipped operator-facing log text. The precedence warning now renders package 'crm' where it rendered package "crm"; the plugin's bootstrap warning is byte-identical for every input its callers can produce.

Consumer sets, re-derived rather than copied

printServerReady — 8 importers: 1 production (commands/serve.ts) and 7 test files, one of them new here. collectAutomationSummary — defined and called in commands/serve.ts, imported by exactly 2 test files, no production consumer outside its own module.

Clause-2

Declared no at dispatch, and it holds: describeFlowContender and resolveFlowPrecedence semantics are untouched, no test case moved, and the diff changes operator prose only.


Generated by Claude Code

…12563)

One event — a flow name claimed by more than one definition — was described to
an operator in three places, each with its own private `const describe` beside
the log call. Nothing held them equal, and two axes had already drifted:
`flow-precedence.ts` rendered `package "crm"` while the other two rendered
`package 'crm'`, and the two engine copies interpolated a bare `undefined`
where the CLI copy rendered a real fallback.

`describeFlowContender` sat exactly where a shared renderer would live and read
like one, but it is a CLASSIFIER with no prose in it — so each caller wrote its
own sentence while an export that looked reusable sat beside them. That is the
durable part of the defect: a missing renderer is a gap someone fills, a
misleading one is a gap that keeps getting re-filled privately.

The two copies in this package now call one exported `renderFlowContender`, and
`describeFlowContender`'s own docs say what it is not. Both spellings were
decided on measurement:

- Single quotes, measured against this package rather than voted across the
  three copies: 203 single-quoted interpolations in operator prose under
  `service-automation/src` against 3 double-quoted, one of which was this
  phrase. The sentence already single-quotes the flow name beside it.
- A named fallback instead of `package 'undefined'`. This package's callers
  cannot reach that branch today (`isCodeArtifactBody` is false on a falsy
  `_packageId`) — but that is a property of today's callers, not of an
  exported function.

The third copy, in `@objectstack/cli`'s startup banner, deliberately keeps its
own structural rendering: that package takes no static value import of
`@objectstack/service-automation`, and its engine reads are feature-detected so
a host on an older automation package still boots its banner. It is held equal
by a test-only agreement pin instead, which is red in both directions.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-automation/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-automation/src/index.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 7c0d0c39552b0eb4aa5c324d183f5eb2105e498epackageMentionDocs.

Which tree this was computed on

This run read content/docs from 651ebf1578b346058e3e8826691b91f336d95f09 — the merge of head ba3a391b1ec8875775cfc8f436c5e8ac8d05e126 into base 7c0d0c39552b0eb4aa5c324d183f5eb2105e498e, 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 651ebf1578b346058e3e8826691b91f336d95f09 && git checkout 651ebf1578b346058e3e8826691b91f336d95f09
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7c0d0c39552b0eb4aa5c324d183f5eb2105e498e ba3a391b1ec8875775cfc8f436c5e8ac8d05e126 && git checkout -B drift-repro 7c0d0c39552b0eb4aa5c324d183f5eb2105e498e && git merge --no-ff ba3a391b1ec8875775cfc8f436c5e8ac8d05e126

node scripts/docs-audit/affected-docs.mjs --json 7c0d0c39552b0eb4aa5c324d183f5eb2105e498e

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

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants