Skip to content

fix: normalize interaction response wire shapes#1114

Open
thymikee wants to merge 2 commits into
mainfrom
codex/issue-1111-interaction-response
Open

fix: normalize interaction response wire shapes#1114
thymikee wants to merge 2 commits into
mainfrom
codex/issue-1111-interaction-response

Conversation

@thymikee

@thymikee thymikee commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

Closes #1111.

Normalizes press/fill/longpress response data behind buildInteractionResponseData so all typed interaction wire payloads use one envelope: shared target/coordinate/evidence fields plus a small per-command extension. Updates MCP output schemas and adds provider-backed contract tests for @ref, selector, and point paths.

Design Decision

The wire response envelope is:

targetKind, canonical x/y when known, referenceWidth/referenceHeight when a runtime snapshot frame exists, normalized identity (ref without @ or selector), optional refLabel, selectorChain, targetHittable, hint, warning, evidence, and message.

Command extensions:

Command Target kind Before inventory New wire shape
press @ref Runtime visualization shape with x/y, frame, ref, refLabel, selectorChain, optional targetHittable/hint/evidence/button/message; backend passthrough could leak default tap fields and override canonical coordinates. Shared ref envelope plus optional non-primary button; backend internals/default tap fields are removed from wire.
press selector Runtime selector shape with x/y, frame, selector metadata, and optional evidence; direct selector-style data had no durable discriminant. Shared selector envelope with targetKind: "selector" and selector identity.
press point x/y plus possible backend default tap fields; no target identity/discriminant. Shared point envelope with targetKind: "point" and canonical coordinates.
fill @ref Historical backend/ref shape; sparse backend results could omit canonical x/y, text, and generated message. Shared ref envelope plus text, delayMs, warning/evidence when present, and canonical resolved coordinates.
fill selector Either runtime selector visualization shape or direct iOS selector payload with only selector/point/text metadata. Shared selector envelope; direct iOS selector still lacks selectorChain/frame until that path has a runtime snapshot, but now carries targetKind and selector identity.
fill point Point/text payload with no target discriminant and possible backend passthrough noise. Shared point envelope plus text and delayMs.
longpress @ref Runtime ref visualization shape plus durationMs/gesture; backend timing internals could leak. Shared ref envelope plus durationMs and gesture: "longpress".
longpress selector Runtime selector visualization shape plus durationMs/gesture; backend timing internals could leak. Shared selector envelope plus durationMs and gesture: "longpress".
longpress point x/y, durationMs, gesture, and message; no target discriminant. Shared point envelope plus durationMs and gesture: "longpress".

Migration Notes

This intentionally makes breaking wire-shape changes instead of preserving accidental shims:

  • fill @ref now returns the canonical envelope instead of the historical backend-only sparse shape.
  • All interaction responses include targetKind.
  • Runner uptime internals, sequence arrays, and default tap noise (count: 1, intervalMs: 0, holdMs: 0, jitterPx: 0, doubleTap: false) are removed from response data.
  • Backend result fields no longer override canonical target identity or resolved coordinates in the wire response. Internal action results still retain backend data where needed for recording.
  • ref remains normalized without the @ prefix.

No docs/skills were changed; this is a command wire/schema/test update, not workflow guidance.

Touched-file count: 7.

Validation

  • node ./node_modules/oxfmt/bin/oxfmt --write ...
  • ./node_modules/.bin/tsgo -p tsconfig.json
  • ./node_modules/.bin/oxlint . --deny-warnings
  • ./node_modules/.bin/vitest run --project unit src/daemon/handlers/__tests__/interaction.test.ts
  • ./node_modules/.bin/vitest run --project interaction-contract test/integration/interaction-contract/interaction-response-shape.contract.test.ts
  • ./node_modules/.bin/vitest run --project interaction-contract (43 tests)
  • ./node_modules/.bin/vitest run --project provider-integration (95 tests)
  • ./node_modules/.bin/vitest run --coverage --reporter=dot (386 files, 3,397 tests)

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.5 MB 1.5 MB +311 B
JS gzip 491.8 kB 491.9 kB +81 B
npm tarball 597.4 kB 597.4 kB +63 B
npm unpacked 2.1 MB 2.1 MB +311 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 25.3 ms 25.4 ms +0.0 ms
CLI --help 48.6 ms 48.9 ms +0.4 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/interaction.js +13.3 kB +4.4 kB

@thymikee

thymikee commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

#1114 is blocked on the Fallow Code Quality job. All other reported checks are passing.

The actionable gate findings are:

  • src/daemon/handlers/interaction-touch-response.ts:179 buildTouchMessage is newly high-complexity in the changed-file scope.
  • src/daemon/handlers/__tests__/interaction.test.ts still has large/high-complexity retained test bodies around the changed scope.

I asked the worker to reduce the new complexity/size findings, rerun pnpm check:fallow --base origin/main plus focused tests, and report back before re-review.

@thymikee

thymikee commented Jul 5, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed after the Fallow fix at bd4cdd8c034e2ffdcd1b3416180ffdcd01f8a7b5.

No blockers found. The PR now has:

  • required issue design: consistent typed interaction response shapes across commands #1111 shape inventory + migration notes in the PR body
  • canonical interaction wire response construction through buildInteractionResponseData
  • MCP output schema alignment for the new shared envelope
  • provider-backed contract coverage for press/fill/longpress across @ref, selector, and point paths
  • all reported checks green, including Fallow, provider integration, coverage, and Swift runner compile

I attempted the requested independent claude -p pass for this codex/ branch, but the local Claude CLI is not logged in in this environment. Based on direct review plus the green CI/provider evidence, this is ready for maintainer judgment.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 5, 2026
@thymikee

thymikee commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Follow-up from the latest review: this PR needs to be stacked after #1106 before it can be merge-ready.

  • The branch currently lacks feat: --settle returns the settled diff in the interaction response (#1101) #1106's settleExtra / settleRefsGeneration path. If this rewrite merges out of order, it can drop --settle wire fields. Please rebase/stack on feat: --settle returns the settled diff in the interaction response (#1101) #1106 and thread settle data through buildTouchPayload.
  • The wire/MCP shape change is intentional but breaking. Please keep the old-to-new field map in the PR body/docs and extend contract coverage across click/fill/press, verify/evidence, warning + stale-ref composition, and non-iOS platform responses.
  • Please check hidden behavior changes in the response builder: old merge precedence was extra -> data, while the new payload builder uses data -> extra; the sanitizer also drops meaningful defaults such as count: 1, intervalMs: 0, and holdMs: 0.

I am removing ready-for-human until the #1106 sequencing blocker and contract/migration coverage are handled.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 6, 2026
@thymikee

thymikee commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Sequencing update after #1106: #1106 has left the open PR set and latest main now contains those settle changes, but this PR is currently DIRTY (head bd4cdd8c034e2ffdcd1b3416180ffdcd01f8a7b5, base 83d54614d849346987d3222709fd458737c9d72a).

Please rebase/refresh #1114 onto current main, then verify the normalized interaction response path preserves the now-merged --settle wire fields and refs generation before re-review.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

design: consistent typed interaction response shapes across commands

1 participant