refactor: reserve the selector engine behind the pipeline owner

Review of #1744 (three blockers).

**Listing rows no longer claim stages they cannot run.** `find <q> list`
resolves to a candidate SET, so promotion, the off-screen guard and a poll
budget have nothing to apply to — a listing has no single element to retarget,
keep on screen, or wait for. `readList` now declares only the two stages a
listing executes (`SelectorListPolicy`: resolution + occlusion), and the
narrower shape is load-bearing: `runNodePipelineStages` and `selectorPollBudget`
take the full row, so handing them a listing row is a compile error rather than
a silently skipped stage. Pinned with `@ts-expect-error` — widening `readList`
makes the directives unused and fails the typecheck.

**The engine door is a specifier, not a symbol.** R17's regex could not see a
namespace import, a re-export, or a deferred `import()`, none of which mention
the symbol it matched. The two engine entries moved to
`@agent-device/selectors/engine`, and R19 enforces over the resolved import
graph, where every one of those forms is the same edge. Proven on the
repo-wide scan by planting each form into a shipped route: namespace import,
dynamic import, and `export *` laundering all come back red.

`resolveImportEdges` drops an edge whose specifier resolves to nothing, so a
specifier rule goes quiet — not red — if the subpath is ever retired. The gate
now says that out loud instead of scanning clean.

**R19, not R17.** #1750 allocates R17/R18. Verified free against origin/main
and that PR's diff, then validated by real merges in both directions: the
uniqueness gate passes either way and the three ids stay distinct.

The gate itself is new (`scripts/layering/rule-ids.ts`): two branches taking one
free number do not conflict in git, so nothing caught R17 twice. Matching whole
string literals is what separates a declaration from prose that names a rule,
and it is what let the gate see #1750's `const RULE = '…'` shape — the first
version missed it and would have been vacuous. `main`'s two pre-existing
collisions (R11, R13) are listed as known, not pinned by equality, so #1750
lands in either order without breaking this.

Also: the root façade now exposes no resolver at all, and its surface test
pins both doors.

Author: thymikee

package.json

@@ -130,7 +130,7 @@
     "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/platform-packages.test.ts scripts/check-affected/run.test.ts",
     "check:coverage-changed": "node --experimental-strip-types scripts/coverage-changed/run.ts",
     "check:coverage-changed:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/coverage-changed/model.test.ts scripts/coverage-changed/run.test.ts",
-    "check:layering": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/layering/model.test.ts scripts/layering/zone-policy.test.ts scripts/layering/daemon-modularity.test.ts scripts/layering/package-boundaries.test.ts scripts/layering/platform-package-policy.test.ts scripts/layering/platform-package-repository.test.ts scripts/layering/platform-package-source-policy.test.ts scripts/layering/device-inventory-cutover-policy.test.ts scripts/layering/logs-runtime-cutover-policy.test.ts scripts/layering/network-runtime-cutover-policy.test.ts scripts/layering/record-runtime-cutover-policy.test.ts scripts/layering/contracts-implementation-policy.test.ts scripts/layering/selector-pipeline-ownership.test.ts scripts/layering/facade-exports.test.ts scripts/layering/bin-alias-fast-path.test.ts && node --experimental-strip-types scripts/layering/check.ts",
+    "check:layering": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/layering/model.test.ts scripts/layering/zone-policy.test.ts scripts/layering/daemon-modularity.test.ts scripts/layering/package-boundaries.test.ts scripts/layering/platform-package-policy.test.ts scripts/layering/platform-package-repository.test.ts scripts/layering/platform-package-source-policy.test.ts scripts/layering/device-inventory-cutover-policy.test.ts scripts/layering/logs-runtime-cutover-policy.test.ts scripts/layering/network-runtime-cutover-policy.test.ts scripts/layering/record-runtime-cutover-policy.test.ts scripts/layering/contracts-implementation-policy.test.ts scripts/layering/selector-pipeline-ownership.test.ts scripts/layering/rule-ids.test.ts scripts/layering/facade-exports.test.ts scripts/layering/bin-alias-fast-path.test.ts && node --experimental-strip-types scripts/layering/check.ts",
     "depgraph": "node --experimental-strip-types scripts/depgraph/build.ts",
     "depgraph:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/depgraph/model.test.ts scripts/depgraph/affected.test.ts",
     "check:production-exports": "fallow dead-code --config fallow-production-exports.json --production --unused-exports --fail-on-issues",

packages/selectors/package.json

@@ -4,7 +4,7 @@
   "private": true,
   "sideEffects": false,
   "type": "module",
-  "description": "Shared selector matching, argument, and replay semantics for agent-device. `.` is string-only; `./ast` is the published parser surface behind `agent-device/selectors`.",
+  "description": "Shared selector matching, argument, and replay semantics for agent-device. `.` is string-only; `./ast` is the published parser surface behind `agent-device/selectors`; `./engine` is the resolve/list surface reserved for the selector-pipeline owner (R19).",
   "dependencies": {
     "@agent-device/ad-script": "workspace:*",
     "@agent-device/contracts": "workspace:*",
@@ -18,6 +18,10 @@
     "./ast": {
       "types": "./src/ast.ts",
       "default": "./src/ast.ts"
+    },
+    "./engine": {
+      "types": "./src/engine.ts",
+      "default": "./src/engine.ts"
     }
   },
   "devDependencies": {

packages/selectors/src/engine.ts

@@ -0,0 +1,24 @@
+/**
+ * The matching engine's own door (#1656).
+ *
+ * "Resolve a selector against a screen" and "list what it matched" are the two
+ * decisions a pipeline policy row exists to qualify: which nodes may be
+ * candidates, whether several matches refuse or collapse, and which structural
+ * stages run around the answer. A caller that reaches these directly gets the
+ * ambiguity contract and silently skips every structural stage — the failure
+ * #1649 caught in the first policy matrix and #1656's review caught in the
+ * second.
+ *
+ * So they live behind a subpath of their own rather than on the root façade,
+ * and R19 selector-pipeline-ownership admits exactly one importer:
+ * `src/core/selector-pipeline.ts`. A specifier is what the import graph
+ * resolves, so namespace imports, dynamic imports, and re-exports are all the
+ * same edge and all equally refused — which a name-shaped check could not say.
+ *
+ * Everything else selectors publishes (parsing, matching, formatting, replay)
+ * stays on the root façade: none of it decides what a row decides.
+ */
+export {
+  listSelectorChainMatches,
+  resolveSelectorChainWithPolicy,
+} from './internal/facade-engine.ts';

packages/selectors/src/index.test.ts

@@ -1,14 +1,15 @@
 import assert from 'node:assert/strict';
+import { resolveSelectorChainWithPolicy } from './engine.ts';
 import { test } from 'vitest';
 import type { SnapshotNode } from '@agent-device/kernel/snapshot';
 import * as selectorsFacade from './index.ts';
+import * as selectorsEngine from './engine.ts';
 import {
   buildSelectorCandidates,
   readReplaySelectorDisplayValue,
   readSelectorExpression,
   resolveRecordedTarget,
   resolveReplaySuggestionCandidate,
-  resolveSelectorChainWithPolicy,
   SELECTOR_RESOLUTION_POLICIES,
 } from './index.ts';
 
@@ -201,16 +202,36 @@ test('replay suggestion resolution and display values stay string-only at the fa
  * preserved semantics — so no fixture-tree assertion can catch a revert
  * (#1715 review). The absence of the symbol is the only observable.
  */
-test('the façade exposes no resolver that bypasses the policy matrix', () => {
+test('the façade exposes no resolver at all, and the engine door exposes only the two entries', () => {
   const exported = Object.keys(selectorsFacade);
-  assert.ok(exported.includes('resolveSelectorChainWithPolicy'));
-  assert.ok(!exported.includes('resolveSelectorChain'), 'knob-taking resolver must stay private');
+  // #1656 moved both engine entries behind `./engine`, which R19 reserves for
+  // the selector-pipeline owner: a route that could reach a resolver from the
+  // root façade would get an ambiguity contract while skipping every
+  // structural stage its policy row declares.
   assert.ok(
-    !exported.includes('findSelectorChainMatch'),
-    'count-only existence lookup must stay private; `is exists` names the readAny row',
+    !exported.includes('resolveSelectorChainWithPolicy'),
+    'resolution belongs to the engine subpath, behind the pipeline owner',
   );
   assert.ok(
-    !exported.includes('selectorResolutionKnobs'),
-    'knob derivation must stay private so a call site cannot rebuild a contract from knobs',
+    !exported.includes('listSelectorChainMatches'),
+    'enumeration belongs to the engine subpath, behind the pipeline owner',
   );
+  assert.deepEqual(Object.keys(selectorsEngine).sort(), [
+    'listSelectorChainMatches',
+    'resolveSelectorChainWithPolicy',
+  ]);
+
+  // Unchanged since #1630, on both surfaces: neither door hands out a knob
+  // resolver a call site could rebuild its contract from.
+  for (const [surface, names] of [
+    ['facade', exported],
+    ['engine', Object.keys(selectorsEngine)],
+  ] as const) {
+    assert.ok(!names.includes('resolveSelectorChain'), `${surface}: knob-taking resolver`);
+    assert.ok(
+      !names.includes('findSelectorChainMatch'),
+      `${surface}: count-only existence lookup; \`is exists\` names the readAny row`,
+    );
+    assert.ok(!names.includes('selectorResolutionKnobs'), `${surface}: knob derivation`);
+  }
 });

packages/selectors/src/index.ts

@@ -1,11 +1,4 @@
-import type { SnapshotState } from '@agent-device/kernel/snapshot';
 import type { Selector } from './internal/parse.ts';
-import type {
-  PolicyResolutionOutcome,
-  SelectorChainMatchList,
-  SelectorMatchOptions,
-} from './internal/public-resolution-types.ts';
-import { resolveSelectorChainWithPolicy as resolveSelectorChainWithPolicyAst } from './internal/resolve-with-policy.ts';
 import {
   checkElementTargetArgs,
   checkGetFormat,
@@ -30,11 +23,7 @@ import {
   IS_PREDICATE_USAGE_HINT,
   normalizeIsPositionals,
 } from './internal/predicates.ts';
-import {
-  listSelectorChainMatches as listSelectorChainMatchesAst,
-  selectorFailureHint,
-  STALE_REF_HINT,
-} from './internal/resolve.ts';
+import { selectorFailureHint, STALE_REF_HINT } from './internal/resolve.ts';
 import {
   findBestMatchesByLocator,
   checkFindArgs,
@@ -81,7 +70,6 @@ export {
   isRoleHintWord,
   isSelectorToken,
   isValidSelectorExpression,
-  listSelectorChainMatches,
   normalizeIsPositionals,
   normalizeSelectorText,
   parseFindArgs,
@@ -226,52 +214,5 @@ function validateSelectorExpression(expression: string): void {
   parseSelectorChain(expression);
 }
 
-/** Public façade wrapper that accepts/returns selector text, never an AST. */
-function listSelectorChainMatches(
-  nodes: SnapshotState['nodes'],
-  expression: string,
-  options: SelectorMatchOptions,
-): SelectorChainMatchList | null {
-  const result = listSelectorChainMatchesAst(nodes, parseSelectorChain(expression), options);
-  return result ? { ...result, selector: result.selector.raw } : null;
-}
-
 export { SELECTOR_RESOLUTION_POLICIES } from './internal/resolution-policy.ts';
 export type { SelectorResolutionPolicy } from './internal/resolution-policy.ts';
-import type { SelectorResolutionPolicy } from './internal/resolution-policy.ts';
-
-/**
- * The façade's ONLY selector-resolution entry (#1630): every native consumer
- * of "resolve a selector against the screen" states its contract by naming a
- * `SELECTOR_RESOLUTION_POLICIES` row, because there is no knob-taking resolver
- * here to state it inline with instead. Accepts selector text and returns
- * selector text — never an AST, in either direction.
- *
- * The return leg is the half that is easy to miss: the parser-side outcome
- * carries the winning `Selector` node inside `resolution`, and returning it
- * unchanged would put a package-private parser object back in every caller's
- * hands through a nested field. The façade's own boundary gate reads exported
- * *names*, so it cannot see that; `selector-wait.ts` reading
- * `outcome.resolution.selector.raw` was the runtime proof it had happened.
- * Flattening here is the same treatment `listSelectorChainMatches` above gives
- * its own selector node (#1589).
- */
-function resolveSelectorChainWithPolicy(
-  nodes: SnapshotState['nodes'],
-  expression: string,
-  policy: SelectorResolutionPolicy,
-  options: SelectorMatchOptions,
-): PolicyResolutionOutcome {
-  const outcome = resolveSelectorChainWithPolicyAst(
-    nodes,
-    parseSelectorChain(expression),
-    policy,
-    options,
-  );
-  if (outcome.kind !== 'resolved') return outcome;
-  return {
-    ...outcome,
-    resolution: { ...outcome.resolution, selector: outcome.resolution.selector.raw },
-  };
-}
-export { resolveSelectorChainWithPolicy };

packages/selectors/src/internal/facade-engine.ts

@@ -0,0 +1,62 @@
+import type { SnapshotState } from '@agent-device/kernel/snapshot';
+import { parseSelectorChain } from './parse.ts';
+import { listSelectorChainMatches as listSelectorChainMatchesAst } from './resolve.ts';
+import { resolveSelectorChainWithPolicy as resolveSelectorChainWithPolicyAst } from './resolve-with-policy.ts';
+import type { SelectorResolutionPolicy } from './resolution-policy.ts';
+import type {
+  PolicyResolutionOutcome,
+  SelectorChainMatchList,
+  SelectorMatchOptions,
+} from './public-resolution-types.ts';
+
+/**
+ * The two engine entries, behind `@agent-device/selectors/engine` (#1656).
+ * Both are string-in/string-out façade wrappers (#1589): a nested parser node
+ * would reopen the AST boundary invisibly, since the package-boundary gate
+ * reads exported *names* and cannot see into a returned shape.
+ */
+
+/** Public façade wrapper that accepts/returns selector text, never an AST. */
+export function listSelectorChainMatches(
+  nodes: SnapshotState['nodes'],
+  expression: string,
+  options: SelectorMatchOptions,
+): SelectorChainMatchList | null {
+  const result = listSelectorChainMatchesAst(nodes, parseSelectorChain(expression), options);
+  return result ? { ...result, selector: result.selector.raw } : null;
+}
+
+/**
+ * The façade's ONLY selector-resolution entry (#1630): every native consumer
+ * of "resolve a selector against the screen" states its contract by naming a
+ * `SELECTOR_RESOLUTION_POLICIES` row, because there is no knob-taking resolver
+ * here to state it inline with instead. Accepts selector text and returns
+ * selector text — never an AST, in either direction.
+ *
+ * The return leg is the half that is easy to miss: the parser-side outcome
+ * carries the winning `Selector` node inside `resolution`, and returning it
+ * unchanged would put a package-private parser object back in every caller's
+ * hands through a nested field. The façade's own boundary gate reads exported
+ * *names*, so it cannot see that; `selector-wait.ts` reading
+ * `outcome.resolution.selector.raw` was the runtime proof it had happened.
+ * Flattening here is the same treatment `listSelectorChainMatches` above gives
+ * its own selector node (#1589).
+ */
+export function resolveSelectorChainWithPolicy(
+  nodes: SnapshotState['nodes'],
+  expression: string,
+  policy: SelectorResolutionPolicy,
+  options: SelectorMatchOptions,
+): PolicyResolutionOutcome {
+  const outcome = resolveSelectorChainWithPolicyAst(
+    nodes,
+    parseSelectorChain(expression),
+    policy,
+    options,
+  );
+  if (outcome.kind !== 'resolved') return outcome;
+  return {
+    ...outcome,
+    resolution: { ...outcome.resolution, selector: outcome.resolution.selector.raw },
+  };
+}

scripts/layering/check.ts

@@ -210,12 +210,6 @@ function checkContractsImplementationAuthority(
   );
 }
 
-function checkSelectorPipelineOwnership(sources: ReadonlyMap<string, string>): LayeringViolation[] {
-  return selectorPipelineOwnershipViolations(
-    [...sources].map(([path, source]) => ({ path, source })),
-  );
-}
-
 function checkNetworkRuntimeCutover(sources: ReadonlyMap<string, string>): LayeringViolation[] {
   const production = [...sources].map(([file, source]) => ({ path: file, source }));
   return [
@@ -666,7 +660,7 @@ export function main(): number {
     ...checkCycles(edges),
     ...checkLogsRuntimeCutover(sources),
     ...checkContractsImplementationAuthority(sources),
-    ...checkSelectorPipelineOwnership(sources),
+    ...selectorPipelineOwnershipViolations(edges, workspaceSpecifierTargets(repoRoot)),
     ...checkNetworkRuntimeCutover(sources),
     ...checkRecordRuntimeCutover(sources),
     ...checkContractsImplementationAuthority(sources),

scripts/layering/package-boundaries.test.ts

@@ -387,13 +387,17 @@ test('the real tree parses, declares, and passes R11', () => {
   );
   const selectorsPackage = packages.find((pkg) => pkg.name === '@agent-device/selectors');
   assert.ok(selectorsPackage, 'selectors package must exist');
-  // Two subpaths, and the split is the point: `.` is the string-only façade
+  // Three subpaths, and each split is the point: `.` is the string-only façade
   // every in-repo consumer uses, `./ast` is the published parser surface that
   // `agent-device/selectors` has shipped since before the engine moved into
-  // this package. A third subpath, or the AST leaking into `.`, fails here.
+  // this package, and `./engine` is the resolve/list surface reserved for the
+  // selector-pipeline owner (R19, #1656) — a route reaching it skips the
+  // structural stages its policy row declares. A fourth subpath, or the AST
+  // leaking into `.`, fails here.
   assert.deepEqual([...selectorsPackage.exportTargets.keys()].sort(), [
     '@agent-device/selectors',
     '@agent-device/selectors/ast',
+    '@agent-device/selectors/engine',
   ]);
   assert.deepEqual([...selectorsPackage.workspaceDependencies].sort(), [
     '@agent-device/ad-script',

scripts/layering/rule-ids.test.ts

@@ -0,0 +1,53 @@
+import assert from 'node:assert/strict';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { test } from 'node:test';
+import {
+  collectRuleDeclarations,
+  duplicateRuleIds,
+  KNOWN_RULE_ID_COLLISIONS,
+  readLayeringSources,
+} from './rule-ids.ts';
+
+const layeringDirectory = path.dirname(fileURLToPath(import.meta.url));
+
+test('no rule id names two rules beyond the collisions #1750 is renaming away', () => {
+  const declarations = collectRuleDeclarations(readLayeringSources(layeringDirectory));
+  assert.ok(declarations.length > 10, 'expected the layering rules to be discovered');
+  const unexpected = duplicateRuleIds(declarations).filter(
+    (collision) => !KNOWN_RULE_ID_COLLISIONS.includes(collision),
+  );
+  assert.deepEqual(unexpected, [], 'two rules answer to one id; allocate the next free number');
+});
+
+test('a collision is reported with both names, whichever order it lands in', () => {
+  // The failure this gate exists for: two branches both take a free number.
+  // Synthetic ids: a fixture naming real rules would read as an allocation to
+  // anyone grepping for the next free number, which is how the collision this
+  // gate exists for happened.
+  const collision = [
+    { path: 'a.ts', source: "rule: 'R99 first-claimant'" },
+    { path: 'b.ts', source: "const RULE = 'R99 second-claimant';" },
+  ];
+  assert.deepEqual(duplicateRuleIds(collectRuleDeclarations(collision)), [
+    'R99 names first-claimant and second-claimant',
+  ]);
+  assert.deepEqual(
+    duplicateRuleIds(collectRuleDeclarations([...collision].reverse())),
+    ['R99 names first-claimant and second-claimant'],
+    'order of arrival must not change the verdict',
+  );
+});
+
+test('one rule declaring its id from several call sites is not a collision', () => {
+  // R7 and R12 legitimately emit from many places.
+  assert.deepEqual(
+    duplicateRuleIds(
+      collectRuleDeclarations([
+        { path: 'a.ts', source: "rule: 'R98 many-emitters'" },
+        { path: 'a.ts', source: "rule: 'R98 many-emitters'" },
+      ]),
+    ),
+    [],
+  );
+});