Skip to content

fix: preserve native interactions while annotating - #127

Closed
JonSEng wants to merge 1 commit into
kunchenguid:mainfrom
JonSEng:fm/lavish-rightclick-annotation
Closed

fix: preserve native interactions while annotating#127
JonSEng wants to merge 1 commit into
kunchenguid:mainfrom
JonSEng:fm/lavish-rightclick-annotation

Conversation

@JonSEng

@JonSEng JonSEng commented Jul 1, 2026

Copy link
Copy Markdown

Intent

Fix Lavish annotation interaction so native selection, left-click, mouseup, keyboard copy, and copy-first behavior remain available while right-click explicitly offers annotation. Preserve data-lavish-action and details/summary behavior; dismiss the custom menu on outside click, Escape, and nested or viewport scroll. Per captain decision J-716, reject broad iframe clipboard permission and unrestricted artifact-to-chrome clipboard bridges: use the narrowest user-initiated least-privilege copy path, with focused sandbox, clipboard, exact-selection, and dismissal tests. Keep the change dependency-free, preserve the sealed artifacts and installed package, publish a review-ready fork PR, and never merge.

What Changed

  • Changed artifact annotation to open via an explicit right-click context menu action instead of intercepting native selection, left-click, mouseup, and keyboard copy behavior.
  • Added menu handling for copy-first selection flows, outside click, Escape, nested scroll, and viewport scroll dismissal while preserving native controls and data-lavish-action behavior.
  • Updated Lavish guidance/docs and added focused artifact SDK/server interaction coverage for annotation, clipboard, selection, sandbox, and dismissal behavior.

Risk Assessment

✅ Low: The change is well-bounded to annotation interaction handling and documentation, with focused tests covering the new right-click menu, dismissal paths, and clipboard constraints.

Testing

After the initial pnpm command exposed a missing local pnpm executable, I installed project dependencies with npx pnpm, ran the focused and full relevant Node test files, then verified the change end-to-end in a rendered Lavish session: selected text retained its selection, right-click showed Copy selection before Annotate selection, annotation produced a text-range prompt, and the native details summary remained clickable.

  • Evidence: Rendered right-click menu showing Copy selection before Annotate selection (local file: /var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-copy-first-menu.png)
  • Evidence: Rendered queued annotation with native details disclosure open (local file: /var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/queued-selection-and-native-details.png)
Evidence: Chrome DevTools interaction transcript
{
  "menu": {
    "selectedText": "Copy this exact review comment, then right-click it: the custom menu should offer Copy selection first and Annotate selection second.",
    "buttons": [
      "Copy selection",
      "Annotate selection"
    ],
    "menuText": "Copy selection\nAnnotate selection"
  },
  "queue": {
    "detailsOpen": true,
    "queuedPrompt": "Please tighten this selected review sentence."
  },
  "send": {
    "sentQueuedPrompts": true
  }
}
Evidence: Lavish poll output showing delivered text-range annotation prompt
session:
  file: /private/var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html
  status: feedback
dom_snapshot: "uid=2 body \"Annotation interaction verification Copy this exact review comment, then right-c\"\n  uid=3 main \"Annotation interaction verification Copy this exact review comment, then right-c\"\n    uid=4 h1 \"Annotation interaction verification\"\n    uid=1 p \"Copy this exact review comment, then right-click it: the custom menu should offe\"\n    uid=5 div \"Native button remains clickable data-lavish-action control\"\n      uid=6 button \"Native button remains clickable\"\n      uid=7 button \"data-lavish-action control\"\n    uid=8 details \"Disclosure summary stays native Details opened normally.\"\n      uid=9 summary \"Disclosure summary stays native\"\n      uid=10 div \"Details opened normally.\"\n  uid=11 script"
prompts[1]:
  - uid: "1"
    prompt: Please tighten this selected review sentence.
    selector: p#review-text
    tag: text
    text: "Copy this exact review comment, then right-click it: the custom menu should offer Copy selection first and Annotate selection second."
    target:
      type: text-range
      text: "Copy this exact review comment, then right-click it: the custom menu should offer Copy selection first and Annotate selection second."
      selector: p#review-text
      commonAncestorSelector: p#review-text
      start:
        selector: p#review-text
        path[0]:
        offset: 0
      end:
        selector: p#review-text
        path[0]:
        offset: 1
next_step: "Apply the requested changes to /private/var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html. Do not respond to the user just yet. Now you must run `lavish-axi poll /private/var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html --agent-reply \"<message for the user>\"` without --timeout-ms unless the user ended the session. The poll waits silently until the user sends more feedback, ends the session, or reports fresh layout_warnings - never kill it. If your harness limits how long a foreground command may run, run the poll as a background task; if it still gets killed or times out, just re-run it - queued feedback is never lost."
Evidence: Lavish open session output
session:
  file: /private/var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html
  url: "http://127.0.0.1:48587/session/852c9d76fe65a75f?no-gate=1"
  status: opened
next_step: "Do not respond to the user just yet. Now you must run `lavish-axi poll /private/var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html`. This command long-polls until the user sends feedback, ends the session, or the real browser reports layout_warnings from the in-iframe layout audit, and it stays silent the whole time - that is normal, never kill it. If layout_warnings arrive, fix overflow, clipped text, or overlapping unreadable content and re-check before involving the human. Do not pass --timeout-ms during normal agent use. If your harness limits how long a foreground command may run, run the poll as a background task and wait for it to finish; if the poll still gets killed or times out, just re-run it - queued feedback is never lost. After applying feedback, run `lavish-axi poll /private/var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html --agent-reply \"<message for the user>\"` without --timeout-ms to show your response in Lavish Editor and wait for more feedback."

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • pnpm test -- --test-name-pattern &#34;annotation|context menu|selection|native|copy|summary|scroll|sandbox|clipboard&#34; test/artifact-sdk-interaction.test.js test/server.test.js (environment check; pnpm was unavailable)
  • npx --yes pnpm@11.1.1 install --frozen-lockfile
  • node --test --test-name-pattern "annotation|context menu|selection|native|copy|summary|scroll|sandbox|clipboard" test/artifact-sdk-interaction.test.js test/server.test.js
  • node --test test/artifact-sdk-interaction.test.js test/server.test.js
  • Created /var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html, opened it through node bin/lavish-axi.js ... --no-open --no-gate, and drove a headless Chrome DevTools session with /var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/cdp-rightclick-evidence.mjs
  • LAVISH_AXI_STATE_DIR=/var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/state LAVISH_AXI_PORT=48587 node bin/lavish-axi.js poll /var/folders/2p/q31vqlw97qn9d7f0yp4y89g80000gn/T/no-mistakes-evidence/01KWF6C9TRJM770M9FHR3PWXYW/rightclick-annotation-demo.html --timeout-ms 1000
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@JonSEng
JonSEng marked this pull request as draft July 1, 2026 16:56
@JonSEng
JonSEng force-pushed the fm/lavish-rightclick-annotation branch from 83f389e to 72751da Compare July 5, 2026 05:13
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Reviewed the full thread, the diff, current main annotation handlers, and VISION.md.

VISION: Native selection/copy while annotating is closer to "nothing interrupts the human," but swapping left-click annotate for a custom right-click menu (and allow="clipboard-write" on the artifact iframe) is default chrome UX, not a corrective or opt-in fix.

Per-rule

  • Author is a fork (JonSEng); not a kunchenguid last-resort port.
  • Required CI is green: PR must be raised via no-mistakes and build-and-test (ubuntu/macos/windows).
  • Mergeable state is CONFLICTING (dirty); the PR is also still a draft. Current main still annotates on mouseup/click via ignoreNextClick.
  • Auto-merge is not allowed for default-behavior changes.
  • Firstmate is only flagged when MERGEABLE + green required CI + only the product decision remains. Do not flag while CONFLICTING. Do not rebase first.
  • Conflicts are resolved only when a PR is otherwise completely auto-merge-ready. This one is not, so no rebase and no CloudAgent.

Holding. No Firstmate flag.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Helping this existing PR from overlapping issue #94 (do not open a competing PR).

#94 is still unfixed on latest main cc186c208e2c: annotate capture-phase click (src/artifact-sdk.js isNativeInteractiveControl + the click listener) still swallows custom/ARIA dropdowns. Native <summary>/<details> were #100; the residual is [role=button] / menuitem / combobox and an opt-out for plain click handlers — the allowlist extension Kun already accepted on #94.

This PR is held as a default chrome UX change (left-click annotate → custom right-click menu, plus allow="clipboard-write"). That hold stands. The #94 landing that would make this PR the vehicle, instead of a competitor spawning beside it, is covering that passthrough on the existing owner (isNativeInteractiveControl) without swapping the default annotate gesture.

Please don't open a second annotation-rewrite PR against #94.

@simfor99

simfor99 commented Sep 2, 2026

Copy link
Copy Markdown

For context on preserving native browsing while annotating: we found that adding right-click (contextmenu) as an additive annotation trigger rather than swapping left-click avoids changing default chrome UX while solving the underlying problem:

  • Left-click annotate continues to work normally when annotate mode is active.
  • Right-click (contextmenu) on any element or text selection immediately triggers an annotation card at the mouse coordinates or text selection context, without requiring a mode switch.
  • Ctrl/Cmd-click is explicitly passed through in the click handler (if (event.ctrlKey || event.metaKey) return;) so that native OS and browser actions (like opening links in a new tab) are never captured or swallowed.

We implemented and tested this alongside the opt-in start-off mode requested in #311.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Re-triaged for newer-activity after stamp 2026-08-20T19:36:03Z.

What changed since stamp

  • No author push, comment, or review. Tip still 72751daac9468d60e3374dd4c37ef729c0a2cda3 (last author force-push 2026-07-05). Still draft and CONFLICTING (dirty) vs current main ffd7aac / 0.1.63.
  • Newer activity is third-party: @simfor99 commented (2026-09-02) describing an additive contextmenu annotate path that keeps left-click annotate, plus modifier-click passthrough — and opened issue Feature idea: Additive right-click to annotate + modifier-click passthrough #317 for that approach. That is not a change to this PR's tip or intent.

Diff vs current main (inspected)

  • Still swaps left-click / mouseup annotate (ignoreNextClick) for a custom right-click menu (showAnnotationMenu / Annotate element|selection, Copy selection first).
  • Still adds allow="clipboard-write" on the artifact iframe (main today has no clipboard permission on that iframe).
  • Main still annotates on capture-phase mouseup/click with annotationMode = true by default; isNativeInteractiveControl still lacks the annotate on click is not good -- and blocks dropdowns #94 ARIA/role allowlist extension.

VISION per-rule

  • Artifact stays the author's — aligns (served SDK/chrome only; no saved-file divergence beyond existing inject).
  • Interaction beats prose — aligns (annotation interaction).
  • Design chosen, never defaulted — N/A to chrome annotate gesture.
  • Nothing interrupts the human — motive aligns (preserve native selection/copy), but replacing the existing default left-click annotate path with a custom menu + clipboard-write changes default chrome interrupt/privilege behavior.
  • Every token spent on purpose — aligns (focused tests/docs; no poll/token surface widen).
  • Instructions are the product — aligns (README documents the new gesture).
  • Scope — mostly aligns; iframe clipboard-write is a privilege widen vs current main.

contract-class: new-default
Replacing the specified default annotate gesture (and adding always-on iframe clipboard permission) is not restore. Not opt-in. Not auto-mergeable.

Related

Stale / merge / flag

  • 14-day stale close: no — this remains a captain-decision / new-default hold, not a waiting-on-contributor close. (Author quiet since 2026-07-05; Firstmate comments do not reset that clock, but captain holds are not closed under the stale skill.)
  • Auto-merge: no (new-default + draft + CONFLICTING).
  • Rebase / CloudAgent: no (not otherwise completely auto-merge-ready).
  • Firstmate / captain flag: no — still draft + CONFLICTING; product decision is not the only remaining gate.

Holding. No Firstmate flag.

@JonSEng JonSEng closed this Sep 2, 2026
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.

3 participants