fix(selectors): type rect-requiring targets as addressable

[kkkhs]

What

Fixes #3576. Rect-requiring selector resolutions were still typed as plain SnapshotNode, so acting paths had to rely on runtime ref guarantees that TypeScript could not see.

Fix

Add SnapshotAddressableNode for nodes with refs and keep require-rect narrowing behind checked selector pipeline exits, including acting candidate collapse.

Test

Adds selector-pipeline type and ref-less regression coverage for action vs read rows.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cubic-dev-ai]

1 issue found across 14 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/selectors/src/selector-pipeline.ts">

<violation number="1" location="packages/selectors/src/selector-pipeline.ts:242">
P2: These casts claim that `requireRect` proves addressability, but the pipeline checks only `rect`, not `ref`; a backend-issued or caller-provided node can therefore be returned as addressable and reach acting code with an undefined ref. Validate `hasSnapshotRef` whenever the policy requires addressability (including candidate lists), or require an addressable node at the acting boundary instead of asserting it.

(Based on your team's feedback about snapshot ref minting modes.) [23b875ef-7244-47fb-b775-a2b67cebe52b]</violation>
</file>

<sub>Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: These casts claim that requireRect proves addressability, but the pipeline checks only rect, not ref; a backend-issued or caller-provided node can therefore be returned as addressable and reach acting code with an undefined ref. Validate hasSnapshotRef whenever the policy requires addressability (including candidate lists), or require an addressable node at the acting boundary instead of asserting it.

(Based on your team's feedback about snapshot ref minting modes.) [23b875ef-7244-47fb-b775-a2b67cebe52b]

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/selectors/src/selector-pipeline.ts, line 242:

<comment>These casts claim that `requireRect` proves addressability, but the pipeline checks only `rect`, not `ref`; a backend-issued or caller-provided node can therefore be returned as addressable and reach acting code with an undefined ref. Validate `hasSnapshotRef` whenever the policy requires addressability (including candidate lists), or require an addressable node at the acting boundary instead of asserting it.

(Based on your team's feedback about snapshot ref minting modes.) [23b875ef-7244-47fb-b775-a2b67cebe52b]</comment>

<file context>
@@ -187,42 +193,60 @@ async function stageResolvedRow(
+  _policy: TPolicy,
+  node: SnapshotNode,
+): SelectorPipelineNodeForPolicy<TPolicy> {
+  return node as SelectorPipelineNodeForPolicy<TPolicy>;
+}
+
</file context>

[thymikee]

The code in 0c4d1f6 looks good to me, and all 14 checks pass. There are no conflicts. I read the diff only and did not run tsc -b or the tests. I also did not trace whether resolveActionableTouchResolution or classifyActionableTouchCandidates can return a ref-less node on any backend. I relied on the producer-fact JSDoc.

Before this is ready, one design question. pipelineNodeForPolicy is exported from the selectors pipeline and called from src/ to cast the winner of classifyActionableTouchCandidates. That winner can be an equivalence owner outside the rect-filtered match list, so the type fact is minted in two packages. The cast at selector-pipeline.ts:238 is also keyed only on the policy type. It checks neither ref nor rect on the node returned by promoteTarget (which can return an ancestor) or by the offscreen hook. The rule would be: every node that leaves a requireRect row is checked by hasSnapshotRef at the single pipeline exit, and the narrowing stays private to @agent-device/selectors.

Could one checked narrowing inside selectors do this at the same size, for example addressableForPolicy(policy, node)? It would use hasSnapshotRef and throw an internal invariant error on a requireRect row. You would apply it at the exit of runNodePipelineStages and inside listSelectorPipelineMatches by mapping matchedNodes instead of casting the list. The acting-classification step in resolveActionSelector would move behind a pipeline-owned entry, so src/ never imports a narrowing helper and no cast is exported. That would also settle the open P2 thread on the unchecked cast, which still applies.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cubic-dev-ai]

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/selectors/src/selector-pipeline.ts">

<violation number="1" location="packages/selectors/src/selector-pipeline.ts:265">
P2: Passing the filtered candidates as the classifier's tree drops ref-less structural ancestors, so its parent-index walk can reject an otherwise collapsible acting match as ambiguous. Preserve the full tree for ancestry/indexing and apply the ref filter only to selector candidates.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Passing the filtered candidates as the classifier's tree drops ref-less structural ancestors, so its parent-index walk can reject an otherwise collapsible acting match as ambiguous. Preserve the full tree for ancestry/indexing and apply the ref filter only to selector candidates.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/selectors/src/selector-pipeline.ts, line 265:

<comment>Passing the filtered candidates as the classifier's tree drops ref-less structural ancestors, so its parent-index walk can reject an otherwise collapsible acting match as ambiguous. Preserve the full tree for ancestry/indexing and apply the ref filter only to selector candidates.</comment>

<file context>
@@ -215,6 +233,57 @@ export function listSelectorPipelineMatches<const TPolicy extends CandidateSetPi
+    };
+  }
+
+  const classification = classifyActionableTouchCandidates(candidates, matched);
+  if (classification.kind === 'ambiguous') {
+    return { kind: 'ambiguous', selector: list.selector, candidates: classification.candidates };
</file context>

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[thymikee]

This PR is ready. The earlier simplicity question is answered, and the cast is now checked in nodeForPolicy, which throws on a rect-requiring row. The full-ancestry path into classifyActionableTouchCandidates is covered by the new test. I reviewed 0c4d1f6 before and f7a36b4 now. Smoke Tests is still queued, not failed. The diff touches the acting selector route that smoke press and fill flows use, so a failure there would need a look against this diff. All other checks pass.

Not blocking: you can take or leave these. First, pipelineCandidates (https://github.com/callstack/agent-device/blob/f7a36b4/packages/selectors/src/selector-pipeline.ts#L325) drops ref-less nodes from the whole tree. Ref-less nodes are also rect-less, so the existing requireRect && !node.rect filter in internal/find.ts already keeps them from matching. The new filter only removes structural ancestors that chain matching sees on web captures. Could nodeForPolicy own the check at the exit, with any match-level exclusion sitting beside the rect filter? Second, nodeForPolicy (https://github.com/callstack/agent-device/blob/f7a36b4/packages/selectors/src/selector-pipeline.ts#L306) throws a plain Error. If the invariant ever breaks, normalizeError reports an untyped failure with no details. An AppError with an invariant code and the node index in details would fix that.

On evidence: I read the code but did not run tsc -b or the selector tests. The ts-expect-error assertions and the claim that the new tests fail on the old code come from that reading. I did not trace whether the web backend can emit a node that has a rect but no ref. The claim that the filter and the throw cannot fire in production rests on the producer-fact JSDoc. I also did not check whether pruning rect-less ancestors changes selector-chain predicate results on web captures.

Of the open inline threads, the two from cubic-dev-ai no longer apply at this head, so you can resolve them. The first is #3608 (comment): the cast is now checked in nodeForPolicy. The second is #3608 (comment): resolveActionSelectorPipeline passes the full nodes to classifyActionableTouchCandidates, and the new full-ancestry test covers it.

Once Smoke Tests turns green, this can merge.