fix(cli): x g edits source lists syntax-aware; a feature's second entity; no stray db client; --dry-run lists every file; three-part permissions

CHANGELOG.md

@@ -8,7 +8,17 @@ Semver applies from 1.0.0. A breaking change to a documented API needs a major 
 
 ## [Unreleased]
 
-Nothing yet.
+### Fixed
+
+Tier 5 — cli.
+
+- **`x g` no longer corrupts a commented role map.** `x g entity` / `x g resource` / `x g policy` add permissions to `apps/web/shared/roles.ts`, and found each list's end with the first `]` in the raw text and its entries with a quote regex: an apostrophe in a comment ("the firm's") was a string delimiter, so the comments between two of them were rewritten into permissions and the file no longer loaded. Every list a generator grows — a role's `grants`, `definePermissions([…])`, a `defineApi` list, the handle's entity set — is now located on the masked source (`source-list-edit.ts`): a list holding a comment gains rows and no existing byte moves, a comment-free one is re-wrapped the way Biome prints it, a spread entry survives, and the result is read back before it is written. An edit that does not read back as "the same list plus the new entries" is `X_PERMISSION_UNGRANTED` / `X_DB_HANDLE_UNREGISTERED` naming the lines to add, never a write.
+- **A permission is declared only where it can be granted.** In a role map with no `admin` role, `x g entity` declared `<table>:read|write|delete` in the first `definePermissions([…])` beside a finding that said nothing was granted — three names no role held. It now declares none of them, and one that any `definePermissions([…])` call already declares is not declared again in the first.
+- **`x g entity <second> --feature <feature>` writes the feature's second entity.** It was `X_GENERATE_CONFLICT` on the first entity's files, with a `--force` that would have overwritten the first table. A slice whose `entity.ts` declares another table now gets `entity-<name>.ts` + `repo-<name>.ts` (and a test beside each), registered on the handle, in the role map and in the admin labels like a first entity; `data.next` names its own table. The same entity again is still the conflict — as is an `entity.ts` whose table the generator cannot read (a constant, an interpolation): only a file that readably declares other tables hosts a sibling.
+- **No `packages/db/src/client.ts` in a db package of the app's own.** The handle was created in any app that lacked the file — including one whose `packages/db/src/index.ts` exports no handle (a schema and a seed over `@ultimat3/db`'s client), where nothing imports it. It is created only where the index names `./client` or there is no index; otherwise `X_DB_HANDLE_UNREGISTERED` on the index says what the generated repo expects, and nothing is written there.
+- **`x g --dry-run` lists every file the run touches.** It listed the files a generator creates and none it edits: the role map, the typed handle, the API index, two manifests and the i18n index changed under a plan that never named them. The dry run now runs the same edits against the plan in memory (`generate-follow-ups.ts` over a disk seam): `data.files` is the run's list, `data.edits` the subset that already exists, `data.refreshes` the committed contracts a real run re-projects, and a finding the run would report (`X_PERMISSION_UNGRANTED`, `X_DB_HANDLE_UNREGISTERED`, `X_ADMIN_RESOURCE_UNWIRED`) makes the dry run `ok: false` too. A real run's list also names `packages/i18n/src/index.ts` when it changed it.
+- **`x g admin:page --permission admin:blog:write` is accepted.** The flag required one colon exactly, which refused every namespaced permission (`admin:<area>:<verb>`) though `Permission` is `${string}:${string}` and `definePermissions()` takes it. Two segments or more; an empty segment, a space or a quote is still `X_CLI_BAD_FLAG` with a runnable example.
+- **The same scan, in the other registrars.** `apps/web/api/index.ts`: a comment between two entries of a `defineApi` list was split into entries, and a new list in a one-line `defineApi({ … });` was written outside the call (now skipped, the file untouched). `packages/db/src/client.ts`: a `}` in a comment closed the entity set. `packages/i18n/src/index.ts`: a `locales: { … }` in a comment was the one edited. `apps/admin/app/admin/admin.ts`: a brace in a string label miscounted the call.
 
 ## 27.6.1 - 2026-10-10
 

packages/cli/src/admin-registration.ts

@@ -4,8 +4,9 @@
 // Performed here; where the admin is not a `defineAdmin({ … })` this can add to, the finding names
 // the two lines.
 
-import { ERROR_DOCS_URL, stripComments } from '@ultimat3/core';
-import { containedPath } from './generate-write';
+import { ERROR_DOCS_URL, maskLiterals, stripComments } from '@ultimat3/core';
+import type { GenerateDisk } from './generate-disk';
+import { appDisk } from './generate-disk';
 import type { Finding } from './output';
 import { holdsKey, withKeyedLine, withSortedImport } from './source-edit';
 import type { GeneratedFile } from './templates/naming';
@@ -70,7 +71,8 @@ export function insertAdminResources(
   let next = source;
   const missing: AdminResourceEntry[] = [];
   for (const entry of entries) {
-    const masked = stripComments(next);
+    // Literals masked too: a brace inside a string label is not one of the call's.
+    const masked = maskLiterals(next);
     const call = /\bdefineAdmin\(\s*\{/.exec(masked);
     const open = call === null ? -1 : call.index + call[0].length - 1;
     const close = open === -1 ? -1 : closeOf(masked, open);
@@ -140,18 +142,17 @@ export function withAdminResources(
 
 /** The overrides among the written paths, each named by what its file and its entity export. */
 async function entriesFor(
-  root: string,
+  disk: GenerateDisk,
   written: readonly string[],
   webModule: string,
 ): Promise<readonly AdminResourceEntry[]> {
   const entries: AdminResourceEntry[] = [];
   for (const path of written) {
     if (!OVERRIDE_PATH.test(path)) continue;
-    const entity = Bun.file(containedPath(root, entityPathOf(path)));
     const entry = entryOf(
       path,
-      await Bun.file(containedPath(root, path)).text(),
-      (await entity.exists()) ? await entity.text() : undefined,
+      (await disk.read(path)) ?? '',
+      await disk.read(entityPathOf(path)),
       webModule,
     );
     if (entry !== undefined) entries.push(entry);
@@ -169,24 +170,24 @@ export interface AdminRegistration {
 export async function registerAdminResources(
   root: string,
   written: readonly string[],
+  disk: GenerateDisk = appDisk(root),
 ): Promise<AdminRegistration> {
-  const web = await readWorkspace(root, WEB_WORKSPACE);
-  const entries = web === undefined ? [] : await entriesFor(root, written, web.name);
+  const web = await readWorkspace(root, WEB_WORKSPACE, disk);
+  const entries = web === undefined ? [] : await entriesFor(disk, written, web.name);
   if (entries.length === 0) return { edited: [], findings: [] };
-  const file = Bun.file(containedPath(root, ADMIN_FILE));
-  if (!(await file.exists())) {
+  const before = await disk.read(ADMIN_FILE);
+  if (before === undefined) {
     return {
       edited: [],
       findings: entries.map((entry) => adminResourceUnwiredFinding(entry, 'does not exist')),
     };
   }
-  const before = await file.text();
   const { source, missing } = insertAdminResources(before, entries);
   const findings = missing.map((entry) =>
     adminResourceUnwiredFinding(entry, 'holds no defineAdmin({ … }) call to add to'),
   );
   if (source === before) return { edited: [], findings };
-  await Bun.write(containedPath(root, ADMIN_FILE), source);
-  const manifest = await declareWorkspaceDependency(root, ADMIN_WORKSPACE, WEB_WORKSPACE);
+  await disk.write(ADMIN_FILE, source);
+  const manifest = await declareWorkspaceDependency(root, ADMIN_WORKSPACE, WEB_WORKSPACE, disk);
   return { edited: manifest === undefined ? [ADMIN_FILE] : [ADMIN_FILE, manifest], findings };
 }

packages/cli/src/api-registration.test.ts

@@ -215,3 +215,66 @@ describe('unit · a binding the index already holds is a finding, never a silent
     expect(findings).toHaveLength(1);
   });
 });
+
+// The list was closed at the first `]` in the raw text and split at every comma, so a comment
+// between two entries became entries: the same scan `generate-grants.ts` made of a role map.
+describe('unit · a commented list is added to, and every comment survives', () => {
+  const COMMENTED = [
+    "import { defineApi } from '@ultimat3/action';",
+    "import * as health from './health';",
+    '',
+    'export const api = defineApi({',
+    "  // jobs: [notThis], — it's prose",
+    '  actions: [',
+    "    // the app's liveness probe, [never] gated",
+    '    health,',
+    '  ],',
+    '  jobs: [',
+    "    // nothing's queued yet, but see tasks[0]",
+    '  ],',
+    '});',
+    '',
+  ].join('\n');
+
+  test('the binding lands after the comment, and the file is otherwise byte-identical', () => {
+    const entries = apiEntriesFor([
+      'apps/web/app/billing/jobs/purge-drafts-job.ts',
+      'apps/web/app/billing/actions/close-period.ts',
+    ]);
+    const { source, skipped } = insertApiEntries(COMMENTED, entries);
+    expect(skipped).toEqual([]);
+    expect(source).toContain(
+      "  jobs: [\n    // nothing's queued yet, but see tasks[0]\n    purgeDraftsJob,\n  ],",
+    );
+    expect(source).toContain(
+      "  actions: [\n    // the app's liveness probe, [never] gated\n    health,\n    closePeriod,\n  ],",
+    );
+    const added = [
+      "import * as closePeriod from '../app/billing/actions/close-period';\n",
+      "import * as purgeDraftsJob from '../app/billing/jobs/purge-drafts-job';\n",
+      '    purgeDraftsJob,\n',
+      '    closePeriod,\n',
+    ];
+    expect(added.reduce((text, row) => text.replace(row, ''), source)).toBe(COMMENTED);
+    expect(insertApiEntries(source, entries).source).toBe(source);
+  });
+});
+
+// A new list is written on the row after the list before it — and when that list shares its row
+// with the call's own `});`, the row after it is OUTSIDE the call.
+describe('unit · a list that cannot be created inside the call is skipped, never written outside it', () => {
+  test('a one-line defineApi is left byte-identical and the entry comes back', () => {
+    const inline = [
+      "import { defineApi } from '@ultimat3/action';",
+      "import * as health from './health';",
+      '',
+      'export const api = defineApi({ actions: [health] });',
+      'export type Api = typeof api;',
+      '',
+    ].join('\n');
+    const entries = apiEntriesFor(['apps/web/app/billing/jobs/purge-drafts-job.ts']);
+    const { source, skipped } = insertApiEntries(inline, entries);
+    expect(skipped).toEqual(entries);
+    expect(source).toBe(inline);
+  });
+});

packages/cli/src/api-registration.ts

@@ -6,10 +6,13 @@
 
 import { ERROR_DOCS_URL } from '@ultimat3/core';
 import { API_INDEX } from './app-root';
+import type { GenerateDisk } from './generate-disk';
+import { appDisk } from './generate-disk';
 import { containedPath } from './generate-write';
 import type { Finding } from './output';
+import type { SourceList } from './source-list-edit';
+import { appendToList, maskOf, readList, readProperties } from './source-list-edit';
 import { camel } from './templates/naming';
-import { wrapList } from './templates/wrap';
 
 /** One module to import as a namespace and list under a `defineApi` key. */
 export interface ApiEntry {
@@ -56,42 +59,30 @@ export function apiEntriesFor(written: readonly string[]): readonly ApiEntry[] {
 }
 
 /**
- * Every `[...]` entry of a `key: [...]` list in the `defineApi({ ... })` call, and where it sits:
- * `line` is the start of the line holding `key: [`, `start` is just past the `[`, `end` is the `]`.
- *
- * Searched from `from` (the `defineApi({` call), so a `jobs: [` in a comment or an object above the
- * call is never the one edited. `line` is found from the key, never from `start`: in a list already
- * wrapped one entry per line the character AT `start` is the newline after `[`, and a backwards
- * search from there answered the first ITEM's line — the rewrite then nested a second `jobs: [`
- * inside the first and left the old `]` behind.
+ * The `key: [...]` lists of the `defineApi({ ... })` call, read off the MASKED text: a `jobs: [` in
+ * a comment or an object above the call is never the one edited, and a `]` or a comma inside a
+ * comment between two entries ends nothing. (The list was closed at the first `]` in the raw text
+ * and split at every comma — a commented list was rewritten with its comment as an entry.)
  */
-const listOf = (
-  source: string,
-  key: string,
-  from: number,
-): { line: number; start: number; end: number; items: string[] } | undefined => {
-  const open = new RegExp(`\\n([ \\t]*)${key}: \\[`, 'g');
-  open.lastIndex = from;
-  const found = open.exec(source);
-  if (found === null) return undefined;
-  const line = found.index + 1;
-  const start = found.index + found[0].length;
-  const end = source.indexOf(']', start);
-  if (end === -1) return undefined;
-  const items = source
-    .slice(start, end)
-    .split(',')
-    .map((item) => item.trim())
-    .filter((item) => item.length > 0);
-  return { line, start, end, items };
+const listsOf = (source: string): ((key: string) => SourceList | undefined) | undefined => {
+  const masked = maskOf(source);
+  const call = /\bdefineApi\(\s*\{/.exec(masked);
+  if (call === null) return undefined;
+  const properties = readProperties(source, masked, call.index + call[0].length - 1);
+  return (key) => {
+    const property = properties.find((one) => one.key === key);
+    return property === undefined || masked[property.value] !== '['
+      ? undefined
+      : readList(source, masked, property.value);
+  };
 };
 
 /**
  * `source` with each entry imported and listed. The import joins the `import * as` block in sorted
  * position; the binding joins its list, which is created after `actions: [...]` when the call has
- * none. An index that is not the scaffold's shape — no `defineApi({`, no `actions:` list — is
- * returned untouched with the entries in `skipped`, and the `manifest` step's X_JOB_UNREGISTERED
- * names the edit.
+ * none. An index that is not the scaffold's shape — no `defineApi({`, no `actions:` list — or a
+ * list that cannot be edited safely is returned untouched with the entries in `skipped`, and the
+ * `manifest` step's X_JOB_UNREGISTERED names the edit.
  */
 export function insertApiEntries(
   source: string,
@@ -101,28 +92,38 @@ export function insertApiEntries(
   const skipped: ApiEntry[] = [];
   for (const entry of entries) {
     const importLine = `import * as ${entry.binding} from '${entry.specifier}';`;
-    const call = next.indexOf('defineApi({');
-    const actions = call === -1 ? undefined : listOf(next, 'actions', call);
-    if (call === -1 || actions === undefined) {
+    const listOf = listsOf(next);
+    const actions = listOf?.('actions');
+    if (listOf === undefined || actions === undefined) {
       skipped.push(entry);
       continue;
     }
-    const list = listOf(next, entry.key, call);
-    if (list?.items.includes(entry.binding) === true) continue;
+    const list = listOf(entry.key);
+    if (list?.entries.some((one) => one.text === entry.binding) === true) continue;
     if (list === undefined) {
       // In the order `x new` writes — actions, queries, jobs, tasks: after the nearest list
       // before it that the call already holds, which is `actions` at the least.
       const earlier = LIST_ORDER.slice(0, LIST_ORDER.indexOf(entry.key))
-        .map((key) => listOf(next, key, call))
+        .map((key) => listOf(key))
         .findLast((found) => found !== undefined);
-      const after = next.indexOf('\n', (earlier ?? actions).end);
+      const after = next.indexOf('\n', (earlier ?? actions).close);
       const line = `  ${entry.key}: [${entry.binding}],`;
-      next = `${next.slice(0, after + 1)}${line}\n${next.slice(after + 1)}`;
+      const candidate = `${next.slice(0, after + 1)}${line}\n${next.slice(after + 1)}`;
+      // Read back, as every other edit here is: when the earlier list shares its row with the
+      // call's own `});`, "the row after it" is OUTSIDE the call — a property of nothing.
+      const landed = listsOf(candidate)?.(entry.key);
+      if (landed?.entries.some((one) => one.text === entry.binding) !== true) {
+        skipped.push(entry);
+        continue;
+      }
+      next = candidate;
     } else {
-      const items = [...list.items, entry.binding];
-      const indent = /^[ \t]*/.exec(next.slice(list.line))?.[0] ?? '';
-      const rewritten = wrapList(indent, `${entry.key}: [`, items, ']');
-      next = `${next.slice(0, list.line)}${rewritten}${next.slice(list.end + 1)}`;
+      const edited = appendToList(next, list.open, [entry.binding]);
+      if (edited === undefined) {
+        skipped.push(entry);
+        continue;
+      }
+      next = edited;
     }
     if (!next.includes(importLine)) next = withImport(next, importLine);
   }
@@ -236,13 +237,13 @@ export async function indexBindingFindings(
 export async function registerGeneratedPrimitives(
   root: string,
   written: readonly string[],
+  disk: GenerateDisk = appDisk(root),
 ): Promise<readonly string[]> {
   const entries = apiEntriesFor(written);
-  const file = containedPath(root, API_INDEX);
-  if (entries.length === 0 || !(await Bun.file(file).exists())) return [];
-  const before = await Bun.file(file).text();
+  const before = entries.length === 0 ? undefined : await disk.read(API_INDEX);
+  if (before === undefined) return [];
   const { source } = insertApiEntries(before, entries);
   if (source === before) return [];
-  await Bun.write(file, source);
+  await disk.write(API_INDEX, source);
   return [API_INDEX];
 }