Repository navigation
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 19 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
Two CI failures from the first push, both resolved in b97a165:
🤖 Generated with Claude Code |
|
Thanks for the HarmonyOS work. I reviewed 0c92a5b; b97a165 landed while I was reviewing and changes only the smoothstep line and the integration-progress row, so it may clear the two CI failures (Repo Guards freerange at gesture-plan.ts:746, and the uncovered Scope first. The help at cli-help-gestures.ts:36 says the human stroke serves flows that judge trajectory shape, and the PR body names slider and drag verification. A seeded wobble, overshoot jitter and per-request randomness read as imitating a person for a slider or drag check, which is bot-detection territory and needs a decision on whether it belongs in agent-device before more code. Could you state the purpose of the randomized parts? A PanGesture recognizer only needs timed interior samples, so a smaller change may cover the legitimate case: no CLI flag and no contract change, just a sample-carrying lowering in Coordinates. The lowering at touch-plan-lowering.ts:170 writes display pixels straight into Evidence. This is a new device-facing injection path through The two P1 inline threads still apply (the discarded shell result and sleep ordering around events), as do the lower-priority ones. The render test at touch-plan-lowering.test.ts:93 counts lines but not order, so it would pass that sleep bug; asserting down, sleep, moves, sleep, up with no trailing sleep would catch it. |
|
The Coverage job on b97a165 failed on one test unrelated to this diff: 🤖 Generated with Claude Code |
|
At b97a165, the two CI failures caused by the diff are addressed, and the remaining Coverage failure is in platform-apple Two notes on the new commit:
The progress-model row thread no longer applies and can be resolved. |
|
Thanks for the follow-up — the CI fixes at b97a165 are acknowledged, and the smoothstep dedup note is fair (it's a On the randomized parts, the honest answer: we don't have timing telemetry that proves the recognizer needs the wobble, overshoot or per-request randomness — that was a design assumption. The concrete motivation we can point at today is in Concretely, we'll take your smaller change as the direction: no CLI flag and no contract change, a sample-carrying lowering in |
|
Thanks, that answers the simplicity question at b97a165. The smaller direction sounds right: a sample-carrying lowering in |
|
b97a165 now conflicts with main. Please rebase when you push the smaller lowering discussed above; the review notes still apply to whatever lands. |
…tory Gesture pan on HarmonyOS densifies the plan's timed path on the shared 16 ms sampling grid and writes the samples to the touchscreen evdev node. The lowering maps display pixels into the panel's own axis range, which it reads from the MMI service dump because commercial builds deny the input device listing, and refuses the transport for rotated displays. The rendered script paces each sleep before its sample, joins the chain with && under a success marker the host verifies, and releases the contact when the chain fails. Any unavailable transport falls back to the uiInput endpoint swipe and reports the reason under trajectoryFallback. Co-Authored-By: Claude Code <noreply@anthropic.com>
…pe contract Co-Authored-By: Claude Code <noreply@anthropic.com>
…le-private Co-Authored-By: Claude Code <noreply@anthropic.com>
…uirk to hold the first screen Co-Authored-By: Claude Code <noreply@anthropic.com>
b97a165 to
da0af4b
Compare
|
Smaller lowering pushed at da0af4b (branch rewritten, so a range-diff against b97a165 is not meaningful — the flag, the marker plumbing, the generator, the contracts change, and the progress-model exclusion entry are all gone; the diff is 5 files, +950/−2). Against the agreed direction and the standing notes:
Help carries the transport semantics on the gestures quirks line (backend + trajectoryFallback). |
…he panel after a failed injection Co-Authored-By: Claude Code <noreply@anthropic.com>
|
Thanks for the rework. The cross-platform flag, the trajectory generator and the extra input field from the earlier review are gone, and the injection now checks that it landed. At 59a2bb5 the code still has one defect and two gaps, so it is not ready to merge. All 11 checks pass, and there are no conflicts. The next step before merge is live evidence from a HarmonyOS device. An unwritable touch node throws instead of falling back. Take a build where The densify loop is now duplicated across platform packages. The new route has no live proof. The only device run is the denied-listing fallback, so the positive route (readable listing, MMI range, RenderService resolution, Could this be much smaller? The change adds an MMI dump parser, a RenderService parser, a rotation-inferring transform, a success-only memo, a marker script and six fallback reasons, and the only device run takes the first refusal. A smaller shape could be: a shared Two resolved threads still apply and are covered above: no test pans again after a failure, and a stale memo after a reboot can point at a different writable node, so a lost stroke would report success. The unwritable-node case assumes the standard Linux evdev permission model, which is worth confirming on the device. |
|
The sanitized findings note is now on the branch: docs/evidence/harmony-uitest-injection-findings-2026-11-04.md (docs-only commit). It keeps the technical content only — device and harness, the two uitest gaps, the runs A–E table, and the related restrictions — and states how they motivate the pan transport's fallback design; the internal submission record stays out of the repository. |
|
At dfccf76 the only change is the new evidence note, so the earlier review still stands: the unwritable-node fallback, the duplicated densify loop, and live proof of the sendevent route. CI is green and there are no conflicts. The note confirms the gap: the only device denies The resolved threads on the memo drop after a failed injection and the renumbered node after reboot still apply, since that code has not changed. |
dfccf76 to
59a2bb5
Compare
|
Head refreshed to 59a2bb5: the series was reworked into five commits (densified lowering with panel-range and render-transform probing, a sentinel-verified injection chain, injection pacing per spawned process, and the help line folded into the existing pan quirk). An internal evidence note that had landed on the branch was removed from the public branch. 🤖 Generated with Claude Code |
|
Thanks for the note. 59a2bb5 is the same commit as the review at 59a2bb5, so that review still stands: an unwritable touch node should fall back to the |
…x pass Co-Authored-By: Claude Code <noreply@anthropic.com>
|
Evidence committed to the branch: Per the plan I'll propose this last in the landing order and rebase onto #4317's plugin interface if that lands first. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
3 issues found across 4 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="docs/evidence/harmonyos-trajectory-slider-2026-11-08.md">
<violation number="1" location="docs/evidence/harmonyos-trajectory-slider-2026-11-08.md:12">
P3: The device serial is a personal hardware identifier. Replace it with a placeholder such as `<device-serial>` here and in the commands, so the evidence note keeps only the technical content.</violation>
<violation number="2" location="docs/evidence/harmonyos-trajectory-slider-2026-11-08.md:47">
P2: This PASS does not establish that the panel transport ran: the command omits the JSON backend disclosure, and the document says the `uiInput` fallback also moves the slider. Record the successful `backend`/`trajectoryFallback` result or device-side sample evidence, otherwise label this as slider-only evidence.</violation>
<violation number="3" location="docs/evidence/harmonyos-trajectory-slider-2026-11-08.md:56">
P3: `@e11` is not minted in this reproduction: after starting the app, the script presses it without a fresh snapshot, although refs are valid only in the latest output. Add a snapshot step and use its returned ref, or use a stable selector or coordinate action.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| stroke as a densified sample chain through the panel's input node with a | ||
| success sentinel (a denied or failed panel write can never report a completed | ||
| gesture; a reboot-renumbered node re-probes). The pass at 3 px shows the | ||
| sample chain held the drag across its whole length on hardware. |
There was a problem hiding this comment.
P2: This PASS does not establish that the panel transport ran: the command omits the JSON backend disclosure, and the document says the uiInput fallback also moves the slider. Record the successful backend/trajectoryFallback result or device-side sample evidence, otherwise label this as slider-only evidence.
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 docs/evidence/harmonyos-trajectory-slider-2026-11-08.md, line 47:
<comment>This PASS does not establish that the panel transport ran: the command omits the JSON backend disclosure, and the document says the `uiInput` fallback also moves the slider. Record the successful `backend`/`trajectoryFallback` result or device-side sample evidence, otherwise label this as slider-only evidence.</comment>
<file context>
@@ -0,0 +1,62 @@
+stroke as a densified sample chain through the panel's input node with a
+success sentinel (a denied or failed panel write can never report a completed
+gesture; a reboot-renumbered node re-probes). The pass at 3 px shows the
+sample chain held the drag across its whole length on hardware.
+
+## Reproduce
</file context>
| ## Device | ||
|
|
||
| - HUAWEI 畅享 90 Plus, model `JDY-AL50`, HarmonyOS 7.0.0.111 (SP10C00E111R4P3), API 26 | ||
| - Serial `77TYD26826028730`, USB, `hdc shell` as uid 2000 (shell) |
There was a problem hiding this comment.
P3: The device serial is a personal hardware identifier. Replace it with a placeholder such as <device-serial> here and in the commands, so the evidence note keeps only the technical content.
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 docs/evidence/harmonyos-trajectory-slider-2026-11-08.md, line 12:
<comment>The device serial is a personal hardware identifier. Replace it with a placeholder such as `<device-serial>` here and in the commands, so the evidence note keeps only the technical content.</comment>
<file context>
@@ -0,0 +1,62 @@
+## Device
+
+- HUAWEI 畅享 90 Plus, model `JDY-AL50`, HarmonyOS 7.0.0.111 (SP10C00E111R4P3), API 26
+- Serial `77TYD26826028730`, USB, `hdc shell` as uid 2000 (shell)
+- Surface: `com.stringzhao.captchaab`, its slider-puzzle page — a test harness
+ app whose status bar carries a machine-readable verdict row
</file context>
| - Serial `77TYD26826028730`, USB, `hdc shell` as uid 2000 (shell) | |
| - Serial `<device-serial>`, USB, `hdc shell` as uid 2000 (shell) |
|
|
||
| ```sh | ||
| hdc -t 77TYD26826028730 shell "aa start -b com.stringzhao.captchaab -a EntryAbility" | ||
| node bin/agent-device.mjs press @e11 --platform harmonyos --serial 77TYD26826028730 |
There was a problem hiding this comment.
P3: @e11 is not minted in this reproduction: after starting the app, the script presses it without a fresh snapshot, although refs are valid only in the latest output. Add a snapshot step and use its returned ref, or use a stable selector or coordinate action.
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 docs/evidence/harmonyos-trajectory-slider-2026-11-08.md, line 56:
<comment>`@e11` is not minted in this reproduction: after starting the app, the script presses it without a fresh snapshot, although refs are valid only in the latest output. Add a snapshot step and use its returned ref, or use a stable selector or coordinate action.</comment>
<file context>
@@ -0,0 +1,62 @@
+
+```sh
+hdc -t 77TYD26826028730 shell "aa start -b com.stringzhao.captchaab -a EntryAbility"
+node bin/agent-device.mjs press @e11 --platform harmonyos --serial 77TYD26826028730
+node bin/agent-device.mjs gesture pan 165 1084 439 0 900 --platform harmonyos --serial 77TYD26826028730
+node bin/agent-device.mjs snapshot --platform harmonyos --serial 77TYD26826028730 | grep 'ROUND='
</file context>
|
The recorded run in the evidence file does not show which route executed, so the sendevent route still has no hardware proof at df4ecc3. The checks are green, and CI cannot run HarmonyOS. I found no conflicts. The earlier note (#4239 (comment)) is not yet resolved. The evidence file is new, but the unwritable-node fallback is unchanged, and the densify loop is still duplicated. The run at https://github.com/callstack/agent-device/blob/df4ecc3/docs/evidence/harmonyos-trajectory-slider-2026-11-08.md#L24 omits --json, so it prints the same text for both routes. The oracle (slider offset 3px) also passes on a straight swipe, as line 43 says. The PR body and the earlier note report that the JDY-AL50 listing is denied, and that takes the uiInput swipe. Build 7.0.0.111 differs from .109, but nothing shows the listing became readable. The file may record the fallback as a pass for the new route, and the existence and argument order of Unchanged from the earlier review, at https://github.com/callstack/agent-device/blob/df4ecc3/packages/platform-harmonyos/src/input-actions.ts#L226: with a readable /proc/bus/input/devices and an eventN node the shell uid cannot write, resolveHarmonyTouchTransport reports available, the first sendevent fails, the marker is missing, and the code throws COMMAND_FAILED. The memo is dropped, so every later pan probes and throws again. Pan worked through the uiInput swipe before this PR, and it would fail on every call on such a build. That also contradicts the help text. This assumes standard Linux evdev permissions, which I have not seen on HarmonyOS. The rule should be: a transport that cannot deliver the first contact frame is unavailable and falls back like every other refusal, and only a failure after the down frame landed may throw. Please enforce it in renderHarmonyTouchScript with a marker after the down frame, map a missing down marker to a typed touch-node-unwritable reason, and replace the test at input-actions.test.ts:259 with one where the down write fails and the result is the uiInput swipe with that reason. After the fix, please attach the unwritable-node --json output too. densifyCanonicalSamples at https://github.com/callstack/agent-device/blob/df4ecc3/packages/platform-harmonyos/src/touch-plan-lowering.ts#L305 copies the merge-and-interpolate loop in lowerAndroidTouchPlan (https://github.com/callstack/agent-device/blob/df4ecc3/packages/platform-android/src/touch-plan-lowering.ts#L61). The offset merge, segment walk and endpoint rule are the same; only the sampling profile and the controlled-scroll easing differ. A fix to segment selection or endpoint handling in one copy will not reach the other. Could the loop move next to sampleGestureOffsets in contracts/gesture-plan.ts, for example as densifyGesturePath(samples, durationMs, profile, progress?), and both lowerings call it so the sampled-path rule has one owner? The three open inline threads on the evidence file still apply: the missing --json and backend (#4239 (comment), covered by the evidence ask above), the literal device serial (#4239 (comment)), and the missing snapshot before @e11 (#4239 (comment)). I did not run a HarmonyOS device, the unit tests or check:affected. CI status comes from the checks on the PR. Before merge, the --json run with landed points and PanGesture counts must be attached, or the sendevent route dropped. Then the unwritable-node fallback must be fixed and the densify loop shared. |
Summary
HarmonyOS
gesture paninjects a densified sample-carrying trajectory through the touch panel's evdev node when the panel node, its coordinate range, and a portrait display are readable, and falls back to the endpoint swipe otherwise.--jsonnames the transport underbackendand the fallback reason undertrajectoryFallback, so a degraded pan is never a silent straight line. The injection is verified by the script's own success marker, and a reboot that renumbers the panel node re-probes instead of failing forever.Motivation: the uiInput transport only writes straight swipes, and no shell-reachable channel below root expresses arbitrary sample sequences, so flows that judge trajectory shape (slider and drag verification, PanGesture recognition) cannot be driven faithfully.
Validation
trajectoryFallbackwhile the endpoint swipe runs.🤖 Generated with Claude Code