Skip to content

chore: remove unreachable read-only find handling from daemon find handler#1290

Merged
thymikee merged 1 commit into
mainfrom
claude/suspicious-khayyam-e305ec
Jul 16, 2026
Merged

chore: remove unreachable read-only find handling from daemon find handler#1290
thymikee merged 1 commit into
mainfrom
claude/suspicious-khayyam-e305ec

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

handleFindWait and the rest of the read-only action handling in src/daemon/handlers/find.ts were dead code: dispatchFindReadOnlyViaRuntime (called at the top of handleFindCommands) intercepts every read-only find action before the legacy handlers can run. This removes the dead path and the constants it leaves behind. Follows up the empirical observation in #1288 (and the correction on #1270).

Unreachability proof

Every dispatch route converges on the same choke point:

  • The daemon registry maps exactly one command (find) to the find route (registry.tsrunFindHandler in request-handler-chain.ts), whose only handler is handleFindCommands. CLI, MCP, replay, and the test engine all dispatch through this chain; src/compat/maestro's only find reference is export-direction. No other file imports handlers/find.ts.
  • handleFindCommands calls dispatchFindReadOnlyViaRuntime before any action dispatch, so even direct callers (unit tests) hit the interception.
  • Both sides classify the action with the same pure parser (parseFindArgs) on the same positionals, and their isReadOnlyFindAction sets are identical (exists/wait/get_text/get_attrs).
  • For a read-only action, every path inside dispatchFindReadOnlyViaRuntime returns a non-null DaemonResponse: arg-validation errors return responses, createSelectorRuntime returns {ok:false, response} or {ok:true}, and the final toDaemonResponse wrapper catches all errors into a response. A throw (from device resolution) propagates out of handleFindCommands entirely — there is no fall-through.

Therefore after the interception, action ∈ {click, fill, focus, type}: the wait branch, the exists/get_text/get_attrs handlers, attachIssuedRefsGeneration, and the sessionless-find device resolution were unreachable, and findActionRequiresRect was constant true.

Empirical confirmation: planted throw new Error('DEAD_CODE_PROBE_…') at the top of all four read-only handlers — the full find handler suite (16 tests, including the three find … wait tests) stays green, matching the #1288 live evidence that a debug marker in handleFindWait never fired.

Changes

  • Remove handleFindWait, handleFindExists, handleFindGetText, handleFindGetAttrs, attachIssuedRefsGeneration, isReadOnlyFindAction, findActionRequiresRect, and the sessionless resolveTargetDevice/ensureDeviceReady fallback from handlers/find.ts.
  • Constant-fold requiresRect === true: match resolution always requires rects, always filters the root container, always prefers on-screen matches; FindContext.session is now non-nullable.
  • Drop the now-dead scope threading: the find fetcher's snapshotScope was always undefined, so the legacyIosSparse.scope recovery field (only ever populated with undefined, only ever by find) is removed from selector-capture-runtime.ts.

The find … wait/exists/get behavior itself is unchanged — it lives in the selector runtime (selector-read.ts) and its tests (including the wait tests in find.test.ts, which exercise the runtime route) are untouched and green.

Gates

  • pnpm typecheck, pnpm lint, pnpm format
  • pnpm check:fallow, pnpm check:production-exports, pnpm check:layering — no issues ✅
  • pnpm test:unit: 4100/4102 passed; the 2 failures are the known CPU-contention flake in runtime-hints.test.ts (26s under load, passes in isolation in 2.6s) ✅
  • pnpm build + pnpm test:smoke

No live device run: the removal is behavior-preserving by construction (dead code) and the surviving mutating-find paths are covered by the handler suite.

…ndler

dispatchFindReadOnlyViaRuntime intercepts every read-only find action
(exists/wait/get_text/get_attrs) inside handleFindCommands and always
returns a response for them, so the legacy handleFindWait polling loop,
handleFindExists/GetText/GetAttrs, attachIssuedRefsGeneration, and the
sessionless-find device resolution behind them were unreachable
(verified empirically in #1288 and again here by planting throws in all
four handlers — the full find suite stays green).

With only mutating actions (click/fill/focus/type) able to reach the
snapshot path, requiresRect is constant true; fold it and drop the
now-dead scope threading, including the legacyIosSparse recovery scope
field that was only ever populated with undefined.
@github-actions

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.7 MB 1.7 MB -2.0 kB
JS gzip 557.5 kB 556.8 kB -718 B
npm tarball 673.2 kB 672.5 kB -651 B
npm unpacked 2.4 MB 2.4 MB -2.0 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.1 ms 27.3 ms +0.2 ms
CLI --help 58.0 ms 57.1 ms -0.8 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/selector-runtime.js -17 B -35 B
dist/src/record-trace-recording.js -8 B -6 B
dist/src/session.js -8 B -4 B

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed current head against the actual find dispatch route and the #1288/#1270 reachability evidence. The read-only actions always return through ; I found no bypass or remaining caller that needs the removed field. Full current-head CI is green. Ready for human merge review.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed current head 74f7abd0 against the actual find dispatch route and the #1288/#1270 reachability evidence. The read-only actions always return through dispatchFindReadOnlyViaRuntime; I found no bypass or remaining caller that needs the removed legacyIosSparse.scope field. Full current-head CI is green. Ready for human merge review.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 16, 2026
@thymikee
thymikee merged commit 042f1c3 into main Jul 16, 2026
22 checks passed
@thymikee
thymikee deleted the claude/suspicious-khayyam-e305ec branch July 16, 2026 10:48
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-16 10:48 UTC

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

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant