refactor!: always rename flow ids that name their source and target, drop the keep-flow-ids mode

CONTRIBUTING.md

@@ -96,8 +96,7 @@ them on a private corpus without copying it into the repository, are described i
   interface it exercises. Black-box tests through the entries and the CLI are in `test/e2e/`,
   shared helpers in `test/helpers/`; `test/repo-hygiene.test.ts` keeps it that way. Name a test
   after what it tests, never after the work step that added it. The suite also pins the documentation: the README quick start and the
-  examples in `docs/` must match what the CLI prints. `test/helpers/setup.ts` clears `BPMN_KEEP_FLOW_IDS`
-  of your shell before every file, so the tests see the CLI's defaults.
+  examples in `docs/` must match what the CLI prints.
 - `tools/`: benchmark, fuzzer, layout regression, isomorphism check, descriptor generator.
   The README demo (`docs/assets/demo.gif`) comes from the `product-shots` skill of the
   `miragon-brand` plugin ([Miragon/corporate-identity](https://github.com/Miragon/corporate-identity)),

docs/commands.md

@@ -45,12 +45,8 @@ keys are in [Syntax and output](reference.md).
 | `--force` | write despite a lossy import, new validation errors, or an existing file (`new`) |
 | `--backup` | copy `<file>` to `<file>.bak` first |
 | `--strict` | exit code 5 when the result has warnings |
-| `--keep-flow-ids` | keep the ids of flows whose ends change; `W_FLOW_ID_STALE` names each id that no longer fits |
 | `--lint-config <path>`, `--no-lint-config` | choose or ignore the project's [bpmnlint](bpmnlint.md) configuration |
 
-`BPMN_KEEP_FLOW_IDS=1` makes `--keep-flow-ids` the default. A result whose ids it kept says so with
-`note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 (...)`.
-
 ## `new`
 
 ```sh

docs/contract.md

@@ -65,10 +65,8 @@ A write rewrites only the elements the change touched, so a git diff shows only
 - Broken links in hand-edited files are not repaired; `bpmn validate` reports them.
 - The Camunda 8 profile checks FEEL syntax, not meaning. The engine finds the rest.
 - A flow id that names its ends is renamed when an edit changes its ends (a splice, a
-  `move`, `set source=`). Where flow ids are API (generated code, tests), `--keep-flow-ids` keeps
-  them, and `W_FLOW_ID_STALE` names each flow whose id no longer fits, with the `bpmn set` that
-  renames it. `BPMN_KEEP_FLOW_IDS=1` makes that the default of a shell
-  ([Environment](reference.md#environment)). Library: `keepFlowIds: true`.
+  `move`, `set source=`). Code that names flow ids (generated code, tests) follows the
+  `renamed:` report.
 
 ::: warning Merges can produce duplicate ids
 Two branches that each add an element of the same name get the same id. The next read refuses the

docs/library.md

@@ -97,7 +97,6 @@ the CLI prints with `--json`. `renderMutation(result)` gives the CLI's text.
 | `layout` | `'auto'` (default), `'incremental'`, `'full'`, `false` | `--layout`, `--no-layout` |
 | `force` | write despite a guard | `--force` |
 | `platform` | `'auto'`, `'c7'`, `'c8'`, `'none'` | `--platform` |
-| `keepFlowIds` | keep flow ids; `W_FLOW_ID_STALE` names each stale one | `--keep-flow-ids` |
 | `validators` | host validators ([Validators](validators.md)) | none |
 | `conventions` | declared id conventions (below) | the `.bpmnlintrc` |
 

docs/ops.md

@@ -100,12 +100,6 @@ creates (new node -> old successor), so the same batch can give it a condition:
 }
 ```
 
-## Flow ids in a batch
-
-With `apply --keep-flow-ids` a flow the file had keeps its id through the whole batch unless an op
-removes it, and the result names stale ids with `W_FLOW_ID_STALE` instead of renaming them.
-A `move` puts the moved node's out-flow back at its new place, so a flow between nodes moved
-together stays theirs.
 
 ## A full example
 

docs/reference.md

@@ -27,8 +27,7 @@ Hashes and running numbers are never copied. An unknown id (`E_NOT_FOUND`) comes
 were probably meant.
 
 A flow whose id names its ends in this form is renamed when an edit changes them (`renamed:` in the
-result); an older flow id (`Flow_AToB`) keeps its id. `--keep-flow-ids` keeps the id and reports
-`W_FLOW_ID_STALE` with the `bpmn set` that renames it.
+result); an older flow id (`Flow_AToB`) stays as it is.
 
 Names become ASCII words (`Prüfung buchen` gives `userTask_pruefungBuchen`), at most 40 characters
 of them: a longer name loses its last words, and a single longer word is cut at 40. An id has at
@@ -167,14 +166,9 @@ fix: `bpmn kinds --section errors`. `-` as the file reads the model from stdin.
 
 | variable | effect |
 | --- | --- |
-| `BPMN_KEEP_FLOW_IDS` | `1`, `true`, `yes`, `on`: every mutating command keeps flow ids, as with `--keep-flow-ids` |
 | `BPMN_LAYOUT_DEBUG` | layout diagnostics on stderr |
 | `BPMN_DEBUG` | the stack trace of an internal error |
 
-Where `BPMN_KEEP_FLOW_IDS=1` changed a result, the result says so, since the reader may not see
-the variable: `note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 (--no-keep-flow-ids renames flows after
-their new ends for one command)`.
-
 ## Quoting
 
 ::: warning Expressions in single quotes

docs/testing.md

@@ -25,12 +25,6 @@ bugs are open, a random campaign would hit them by chance. Layout quality is
 gated deterministically by the property test's fixed walks, the regression
 budget and `bench --compare`.
 
-None of the layers depends on the shell's `BPMN_KEEP_FLOW_IDS`: the unit
-tests clear it before every file (`test/helpers/setup.ts`), so the CLIs they spawn
-rename flows as by default, and the fuzzer, the benchmark, the layout
-regression and the demo run the CLI without it (`CLI_ENV` in
-`tools/fuzz/lib/setup.mjs`). A test that needs it sets it for its own spawn.
-
 ## Invariants
 
 The fuzzer and the property test check after every step
@@ -632,10 +626,9 @@ conventions, else element-id-naming's defaults, whatever ids the file has; bpmn-
 the types the rule does not check; the guard that `DEFAULT_ID_CONVENTIONS` equals the plugin's
 defaults), `test/domain/document/ids/speaking-ids.test.ts` and
 `test/domain/document/ids/id-style.test.ts` (renamed flows and their DI, also after an end got
-another id earlier in the batch; a flow id in an older form that keeps its id),
+another id earlier in the batch; a flow id in an older form that stays as it is),
 `test/domain/document/ids/id-quality.test.ts` (cuts to 64 characters, collisions,
-transliteration), `test/domain/editing/keep-flow-ids.test.ts` (flow ids kept with
-`--keep-flow-ids`, `W_FLOW_ID_STALE`, an end renamed or named earlier, `--flow-id` on a splice),
+transliteration),
 `test/domain/validation/conventions-*.test.ts` (declared conventions and the reader of the
 `.bpmnlintrc`), `test/domain/batch/batch-aliases.test.ts` (aliases in `apply`, on the command
 line too), `test/domain/document/not-found.test.ts` (the candidates of `E_NOT_FOUND`),

plugins/bpmn-cli/skills/bpmn/SKILL.md

@@ -119,19 +119,7 @@ installed version. Ask it instead of guessing:
    - `created` / `changed` / `removed` lines and `renamed: A -> B` (a flow whose
      id named its ends is renamed when a splice changes them: use the new id)
    - when flow ids are API (code generated from the model, e.g. bpmn-to-code,
-     or tests that name them), keep them: pass `--keep-flow-ids` to every
-     mutating command and `apply`, or ask the user to set `BPMN_KEEP_FLOW_IDS`
-     to `1` in the `env` of their Claude Code settings (an inline
-     `BPMN_KEEP_FLOW_IDS=1 bpmn ...` is not pre-approved). Then
-     `warning W_FLOW_ID_STALE <flowId>` names a flow whose id no longer names
-     its ends: tell the user; rename it (the `bpmn set` of its hint) only
-     together with the code that uses it. A result line
-     `note: flow ids kept: BPMN_KEEP_FLOW_IDS=1 (...)` means the user's
-     setting kept them although you passed no flag, and that made this
-     result differ: a `W_FLOW_ID_STALE` instead of `renamed:`, or a move
-     that put the node's old out-flow back instead of a new flow. An
-     `E_NOT_FOUND` in an `apply` batch whose hint names the variable: use
-     the flow's old id
+     or tests that name them), tell the user each `renamed:` so the code follows
    - `warning ...` lines are only the warnings **this change added**, each with
      a hint that is the fixing command; `resolved:` what it fixed; old warnings
      are only counted
@@ -306,7 +294,7 @@ Every command, selector and fix: [reference/formatting.md](reference/formatting.
 | `E_NO_ROOM` | Run the `bpmn space ...` command of the hint (checked to make the room), then place again; in a batch after other ops the hint names the space op to put before the place op; without one, move the shapes it names first. |
 | `E_UNKNOWN_ALIAS` | Define the alias (`as` / `flowAs` / `outAs` / `joinAs` / `refAs`) in an earlier op of the batch. |
 | `E_IMPORT_LOSSY` | The file has content the reader would drop. Show the user `bpmn validate` output and ask before `--force`; do not repair the XML by hand. |
-| `E_USAGE` | Bad flag, ops key or `BPMN_KEEP_FLOW_IDS` value: the message names it; see `bpmn <command> --help`. A bad `BPMN_KEEP_FLOW_IDS` is in the user's settings: tell them; `--keep-flow-ids` / `--no-keep-flow-ids` decide for one command. |
+| `E_USAGE` | Bad flag or ops key: the message names it; see `bpmn <command> --help`. |
 
 Every code with its fix: `bpmn kinds --section errors`.
 

plugins/bpmn-cli/skills/bpmn/reference/recipes.md

@@ -76,8 +76,7 @@ printf '%s' '[
 The dry run prints exactly what the write will do: the created ids, the flow
 that was split (`flow` splices into it; it keeps its name and condition, and a
 flow whose id named its old ends is renamed, here
-`flow_checkOrderToOrderOk -> flow_checkOrderToCheckCreditLimit`, unless flow
-ids are kept with `--keep-flow-ids`), what the
+`flow_checkOrderToOrderOk -> flow_checkOrderToCheckCreditLimit`), what the
 layout `placed` / `moved` / `rerouted`, and `layout quality: a -> b` with any
 `added:` problem. When it looks right, run the same batch without `--dry-run`:
 

src/adapters/in/cli/MutationCommand.ts

@@ -9,7 +9,7 @@ import type { MutationResult } from '../../../domain/batch/types.js';
 import { LAYOUT_MODES, type LayoutMode } from '../../../domain/diagram/types.js';
 import { RENDER_MAX_SIZE, type RenderOptions } from '../../../application/port/in/RenderSvgUseCase.js';
 import { changeHighlight } from '../report/utils/highlight.js';
-import { type MutationReport, type MutationSummary } from '../report/command-report.js';
+import { type MutationReport } from '../report/command-report.js';
 import { RenderReport } from './RenderReport.js';
 import { isBpmnError, usageError } from '../../../domain/document/utils/errors.js';
 import { CliCommand, type RawOpts } from './CliCommand.js';
@@ -33,8 +33,6 @@ export interface OutputOptions {
   summary?: boolean;
   /** --render <file>: draw the result to this image */
   render?: string;
-  /** BPMN_KEEP_FLOW_IDS when it, not a flag, kept the flow ids (the result then says so) */
-  keepFlowIdsEnv?: string;
 }
 
 export type MutationFlags = FileMutationOptions & OutputOptions;
@@ -61,9 +59,7 @@ export abstract class MutationCommand extends CliCommand {
       .option(...commonOption('--force'))
       .option(...commonOption('--backup'))
       .option(...commonOption('--show'))
-      .option(...commonOption('--strict'))
-      .option(...commonOption('--keep-flow-ids'))
-      .option(...commonOption('--no-keep-flow-ids'));
+      .option(...commonOption('--strict'));
   }
 
   protected withTriggerOptions(cmd: CommandType): CommandType {
@@ -80,7 +76,6 @@ export abstract class MutationCommand extends CliCommand {
   }
 
   protected mutationOptions(options: RawOpts): MutationFlags {
-    const { keep: keepFlowIds, env: keepFlowIdsEnv } = this.keepFlowIdsOption(options);
     return {
       json: !!options['json'],
       pretty: !!options['pretty'],
@@ -93,8 +88,6 @@ export abstract class MutationCommand extends CliCommand {
       show: !!options['show'],
       strict: !!options['strict'],
       ...(options['render'] !== undefined ? { render: String(options['render']) } : {}),
-      ...(keepFlowIds !== undefined ? { keepFlowIds } : {}),
-      ...(keepFlowIdsEnv !== undefined ? { keepFlowIdsEnv } : {}),
       ...this.lintOptions(options),
     };
   }
@@ -131,7 +124,7 @@ export abstract class MutationCommand extends CliCommand {
         },
       );
     } catch (err) {
-      throw this.withFile(this.keptIdsHint(err, commands, opts.keepFlowIdsEnv), file);
+      throw this.withFile(err, file);
     }
   }
 
@@ -162,20 +155,15 @@ export abstract class MutationCommand extends CliCommand {
     await this.printMutation(result, opts, xmlOut ? this.cli.io.stderr : this.cli.io.stdout);
   }
 
-  /**
-   * --strict exits 5 on any warning; flow ids BPMN_KEEP_FLOW_IDS kept add a note,
-   * also to --summary.
-   */
+  /** --strict exits 5 on any warning. */
   private async printMutation(result: MutationResult, opts: OutputOptions & { show?: boolean; dryRun?: boolean }, out: CliStream): Promise<void> {
-    const note = this.keptByEnvNote(result, opts.keepFlowIdsEnv);
-    if (note) result.changes.note(note);
     const report = new MutationReportBuilder(result).report();
     const shot = opts.render ? await this.renderResult(result.xml, report, opts.render) : undefined;
     const shotText = shot ? `\n${shot.lines.join('\n')}` : '';
     if (opts.summary) {
       const summary = new MutationSummaryBuilder(report).summary();
-      if (opts.json) this.cli.printJson({ ...summary, ...(note ? { notes: [note] } : {}), ...(report.view ? { view: report.view } : {}), ...(shot ? { render: shot.json } : {}) }, opts.pretty, out);
-      else this.cli.print(this.summaryText(summary, { dryRun: opts.dryRun, advisory: advisoryOf((report).validation.validators) }, note) + shotText + (report.view ? `\n\n${new ModelViewText(report.view).text()}` : ''), out);
+      if (opts.json) this.cli.printJson({ ...summary, ...(report.view ? { view: report.view } : {}), ...(shot ? { render: shot.json } : {}) }, opts.pretty, out);
+      else this.cli.print(new SummaryText(summary, { dryRun: opts.dryRun, advisory: advisoryOf((report).validation.validators) }).text() + shotText + (report.view ? `\n\n${new ModelViewText(report.view).text()}` : ''), out);
     } else if (opts.json) this.cli.printJson(shot ? { ...report, render: shot.json } : report, opts.pretty, out);
     else this.cli.print(new MutationText(report, { dryRun: opts.dryRun }).text().replace(/\n$/, '') + shotText, out);
     if (opts.strict && new MutationReportBuilder(result).warnings().length) this.cli.exitCode = 5;
@@ -198,45 +186,6 @@ export abstract class MutationCommand extends CliCommand {
     }
   }
 
-  /**
-   * A note when BPMN_KEEP_FLOW_IDS, not a flag, kept flow ids where the default differs:
-   * a stale flow id, or a move that put the moved node's old out-flow back. The variable
-   * sits in a shell or an agent's settings, where whoever reads the result does not see it.
-   */
-  private keptByEnvNote(result: MutationResult, value: string | undefined): string | undefined {
-    if (value === undefined) return undefined;
-    const { created, changed, warnings, relocated } = result.changes;
-    const putBack = relocated.length > 0 && changed.some((change) => change.kind === 'sequenceFlow' && !created.some((creation) => creation.id === change.id));
-    if (!putBack && !warnings.some((warning) => warning.code === 'W_FLOW_ID_STALE')) return undefined;
-    return `flow ids kept: BPMN_KEEP_FLOW_IDS=${value} (--no-keep-flow-ids renames flows after their new ends for one command)`;
-  }
-
-  /**
-   * renderSummary with `note` after the change lines, where the full result has its notes
-   * (the summary prints no notes of its own).
-   */
-  private summaryText(summary: MutationSummary, opts: ConstructorParameters<typeof SummaryText>[1], note: string | undefined): string {
-    const text = new SummaryText(summary, opts).text();
-    if (!note) return text;
-    const { aliases: _aliases, renamed: _renamed, format: _format, ...rest } = summary;
-    const below = new SummaryText({ ...rest, created: {}, changed: [], removed: [] }, opts).text().split('\n').length - 1;
-    const lines = text.split('\n');
-    lines.splice(lines.length - below, 0, `note: ${note}`);
-    return lines.join('\n');
-  }
-
-  /**
-   * With kept flow ids, a missing id may be the one the default gives a flow an earlier op
-   * rewired; the hint says so, as whoever reads the error may not see the variable.
-   */
-  private keptIdsHint(err: unknown, commands: readonly EditCommand[], value: string | undefined): unknown {
-    if (value === undefined || commands.length < 2 || !isBpmnError(err) || err.code !== 'E_NOT_FOUND') return err;
-    const kept = `If it is the id a flow an earlier op gave other ends would be renamed to: flow ids are kept here (BPMN_KEEP_FLOW_IDS=${value}), so that flow kept its old id; use the old id, or --no-keep-flow-ids to rename it after its new ends.`;
-    const hint = err.details.hint;
-    err.details.hint = typeof hint === 'string' && hint ? `${hint} ${kept}` : kept;
-    return err;
-  }
-
   /** Puts the file name into a refusal's runnable command (E_NO_ROOM) and its hint. */
   private withFile(err: unknown, file: string): unknown {
     if (!isBpmnError(err) || typeof err.details['command'] !== 'string' || file === '-') return err;
@@ -258,28 +207,4 @@ export abstract class MutationCommand extends CliCommand {
     if (layout === false) return false;
     return (layout as LayoutMode | undefined) ?? 'auto';
   }
-
-  /**
-   * --keep-flow-ids / --no-keep-flow-ids, else BPMN_KEEP_FLOW_IDS; `env`: the variable's
-   * value when it kept the ids.
-   */
-  private keepFlowIdsOption(options: RawOpts): { keep?: boolean; env?: string } {
-    const flag = options['keepFlowIds'] as boolean | undefined;
-    if (flag !== undefined) return { keep: flag };
-    const keep = this.envSwitch('BPMN_KEEP_FLOW_IDS', 'Set BPMN_KEEP_FLOW_IDS=1 (true, yes, on) to keep flow ids, BPMN_KEEP_FLOW_IDS=0 (false, no, off) or unset it to rename them after their new ends; --keep-flow-ids / --no-keep-flow-ids decide for one command.');
-    if (keep === undefined) return {};
-    return keep ? { keep, env: this.cli.io.env['BPMN_KEEP_FLOW_IDS']!.trim() } : { keep };
-  }
-
-  /**
-   * Unset or empty: undefined; 1 / true / yes / on: true; 0 / false / no / off: false.
-   * Anything else is E_USAGE: a typo must not silently mean on or off.
-   */
-  private envSwitch(name: string, hint: string): boolean | undefined {
-    const value = this.cli.io.env[name]?.trim();
-    if (!value) return undefined;
-    if (/^(1|true|yes|on)$/i.test(value)) return true;
-    if (/^(0|false|no|off)$/i.test(value)) return false;
-    throw usageError(`${name}=${value} is not a switch value`, { hint });
-  }
 }