Skip to content

fix: omit interaction uptime from wire responses#1142

Merged
thymikee merged 2 commits into
mainfrom
fix/interaction-wire-uptime-noise
Jul 7, 2026
Merged

fix: omit interaction uptime from wire responses#1142
thymikee merged 2 commits into
mainfrom
fix/interaction-wire-uptime-noise

Conversation

@thymikee

@thymikee thymikee commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary

Omit runner uptime internals from interaction wire responses.

Filters currentUptimeMs alongside the existing runner timing/default tap-option noise while keeping the unsanitized result available to daemon history and recording telemetry. Extends the interaction response-shape contract with a noisy default tap result matching the Bluesky dogfood report.

Closes #1077.

Validation

  • pnpm --pm-on-fail=ignore format
  • pnpm --pm-on-fail=ignore exec vitest run --project interaction-contract test/integration/interaction-contract/interaction-response-shape.contract.test.ts
  • pnpm --pm-on-fail=ignore typecheck
  • pnpm --pm-on-fail=ignore lint
  • pnpm --pm-on-fail=ignore test:integration:provider (passed outside sandbox after sandbox listen EPERM)
  • pnpm --pm-on-fail=ignore test:coverage (passed outside sandbox after sandbox listen EPERM)
  • pnpm --pm-on-fail=ignore build
  • Live Bluesky iOS check: node bin/agent-device.mjs press @e18 --json --session issue-1077-bluesky --platform ios returned x, y, message, ref/selector metadata, and no gestureStartUptimeMs, gestureEndUptimeMs, currentUptimeMs, count, intervalMs, holdMs, jitterPx, or doubleTap.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.6 MB 1.6 MB +10 B
JS gzip 503.0 kB 503.0 kB +37 B
npm tarball 600.8 kB 600.8 kB +23 B
npm unpacked 2.1 MB 2.1 MB +10 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.0 ms 27.1 ms -0.9 ms
CLI --help 54.9 ms 54.4 ms -0.5 ms

Top changed chunks: no changes in the largest emitted chunks.

@thymikee

thymikee commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Reviewed the diff — the fix is correct and consistent with the existing gestureStartUptimeMs/gestureEndUptimeMs handling: it strips currentUptimeMs from the wire copy while the unsanitized result keeps it, and recording still reads what it needs from the merged interaction result / record-start anchor (not this field). All 8 keys in NOISY_DEFAULT_TAP_RESULT filter cleanly, so assertExactKeys still sees exactly REF_KEYS. No correctness concerns.

Two actionable notes:

  1. Fail-open denylist is the recurring root cause. shouldKeepWireBackendField defaults to return true, so every new runner backend field is client-facing until someone adds a case — this is the 2nd runner internal to leak this way (gesture uptimes, now currentUptimeMs). Worth considering inverting to an allowlist of known wire keys (fail-closed) so the next runner internal doesn't ship as noise by default. The tradeoff is enumerating legitimate passthrough keys across tap/type/longPress/scroll/etc.; if that's too broad to do here, tracking it as tech debt would still help close the class.

  2. Noise stripping is only exercised on the runtime @ref path. The runner-payload branch (direct selector, assertCanonicalDirectSelector) calls the same sanitizeWireBackendData but is only fed a clean payload. Optional: feed a noisy result through one direct-selector test too, since it's a structurally separate call site and this suite's job is nailing the wire shape across every envelope.

@thymikee

thymikee commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Fallow is the remaining blocker on this PR. The failed job reports one complexity finding: src/daemon/handlers/interaction-touch-response.ts:182 shouldKeepWireBackendField (10 cyclomatic, 31.6 CRAP). Please simplify/split that helper and rerun pnpm check:fallow --base origin/main before requesting review.

@thymikee

thymikee commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Rechecked after the Fallow fix. The sanitizer simplification preserves the intended wire filtering, and the direct-selector path now also gets noisy backend data in the response-shape contract. Checks are green; no remaining blockers from this review pass.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 7, 2026
@thymikee thymikee merged commit 9dfebbe into main Jul 7, 2026
21 checks passed
@thymikee thymikee deleted the fix/interaction-wire-uptime-noise branch July 7, 2026 14:53
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-07 14:54 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.

Interaction responses leak runner uptime internals and default-value noise

1 participant