Skip to content

feat(harness): Shift+click extends selection in composer + queue editor (plan #752) - #762

Merged
btipling merged 6 commits into
mainfrom
plan/shift-click-select
Aug 22, 2026
Merged

feat(harness): Shift+click extends selection in composer + queue editor (plan #752)#762
btipling merged 6 commits into
mainfrom
plan/shift-click-select

Conversation

@btipling

Copy link
Copy Markdown
Owner

Shift+click extends selection in the composer + queue-row editor

Closes #752 (implements plan issue #752, source #749).

What this does

The canvas textEntry fields (composer + queue-row editor, which share the same
vendored TextEntryWidget/TextLayoutWidget) today move the caret on click but
never extend a selection on Shift+click — stock web text-field behavior that
was missing. This ships it:

  • Click at A to place the caret, Shift+click at B[min(A,B), max(A,B)]
    is selected and Ctrl/Cmd+C copies that substring (the chord stays
    browser-reserved; the widget's own clipboardTextSet copy path serves it).
  • A plain (non-shift) click still moves the caret and clears the range.
  • Selection uses the existing TEAL text_select fill — no new hex, no EMBER.
  • Touch/mobile has no shift key — 📋 stays the reliable copy path.

Step-1 verification (the plan's required first check)

Verified against the live pin david-vanderson/dvui#4f810ef:
TextEntryWidget.processEvent .mouse handles only .focus — every other
mouse event falls through to TextLayoutWidget.processEvent, whose
.press+.pointer() sets .mouse .down_pt with no shift check, and whose
selMovePre resolves that to a plain moveCursor(ba.byte, false). So upstream
does not extend on Shift+click
— the plan's optimistic "test/doc lock" path did
not apply; a real vendored patch was required.

How it's implemented

native/harness/build.zig — a new applyInvincibleShiftClickPatch, the #647
fail-closed pattern (idempotent invincible: dedicated shift-click marker; every
needle must match exactly once or the build panics). It threads a dedicated
sel_move.shift_click variant through the pinned TextLayoutWidget.zig:

  • union — new shift_click { anchor, down_pt, byte } variant;
  • press handlerme.mod.shift() on a pointer press captures
    anchor = selection.cursor (pre-press caret) + the click point and skips the
    plain .mouse/double-click paths;
  • selMovePre / lineBreak / addTextDone — resolve the click point to a
    byte index during addText and set selection = [min(anchor,byte), max(...)]
    (the async resolution is the only place text rects exist);
  • selMoveText / cursorSeen / bytesNeeded — empty arms for switch exhaustiveness.

Both editable textEntry fields share the vendored widget, so one patch covers
composer and queue-row editor (per the plan). Plain left-click move-caret,
double/triple-click word/line select, and the #647 right-click link-copy path are
untouched.

Copy (plan open question 1): verified that TextEntryWidget.processEvent
already catches the copy keybind → self.copy()clipboardTextSet(sel), and
the harness keymap marks Ctrl/Cmd+C as RESERVED (never handled, falls through to
the widget). So when a range exists, Ctrl/Cmd+C already copies it — no copy
patch needed.

Tests & verification (this is Zig-only — no TS/bridge/protocol change; wasm-int TS suite not a gate here)

  • zig build test-rich — exit 0
  • zig build test-rich-invariants — exit 0
  • zig build harness -Doptimize=Debug — green (full Wasm compile)
  • zig fmt --check build.zig — clean
  • build-harness CI (self-hosted wasm32-freestanding release) is the authoritative Wasm gate — pending on this PR; will confirm green before stopping.
  • The 4 dvui_testing layout tests that print "failed command" in this sandbox
    (composer_layout, model_picker_layout, paint_diff, transcript_split_layout)
    are pre-existing there (verified byte-identical failures on clean main) —
    they use the separate dvui_testing dependency and fail only on missing fallback
    fonts, unaffected by this patch.

Test strategy (plan test row 6): a host dvui_testing hit-test of a
textEntry click point is unreliable — the testing backend uses fallback fonts
(Font Noto Sans not loaded in dvui), so glyph x-extents/byte-position hit-testing
are bogus. Per the plan's own row 6 allowance, the gates are operator smoke
(goals 1/2/3/4) + the build gates above; the #647 right-click regression is covered
by the untouched .right branch + build.

Living docs

  • docs/harness-limits.md — new Keyboard & focus Shift+click chord row
    (range-select semantics, TEAL fill, reserved-chord copy, plain-click clear,
    touch/📋 note) + Transcript copy / paste Composer / queue-row editor
    Shift+click
    row scoping the in-canvas read-only textLayout descendant as a
    child follow-up.
  • AGENTS.md / README.md / SECURITY.md / .env.example — N/A (no agent rule,
    no visitor-facing, no trust-boundary, no new env).

Notes from an escalated implementation (living comment on plan issue #752)

The plan flagged step-1 verification and two open questions. This was escalated
with a single living planning comment (issue #752, amended in place) recording the
verification finding (upstream does not extend), the locked .shift_click
design, and the resolution of both open questions (copy = widget-internal, no
patch; test strategy = operator smoke + build gates).

CI expected-delta note: Zig host suites keep their baseline pass counts (this adds
no new test rows — the new behavior is gated by operator smoke per plan row 6);
the authoritative Wasm gate is build-harness.

…or (plan #752)

adds a dedicated sel_move.shift_click variant to the pinned TextLayoutWidget
via a fail-closed build.zig patch (invincible: dedicated shift-click marker,
needle must match exactly once). On Shift+pointer-press the selection extends
from the pre-press caret to the click point; plain left-click move-caret,
double/triple-click word/line select, and the #647 right-click link-copy path
are untouched. Copy on the reserved Ctrl/Cmd+C falls through to the widget's
own clipboardTextSet copy (no new patch). Docs updated in harness-limits.md.
@btipling btipling self-assigned this Aug 22, 2026
@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
invincible Ignored Ignored Aug 22, 2026 6:08am

Request Review

@btipling

Copy link
Copy Markdown
Owner Author

Implemented plan #752 (source #749). Step-1 verification confirmed upstream dvui (4f810ef) does not extend a selection on Shift+click — a dedicated sel_move.shift_click vendored patch was required (build.zig applyInvincibleShiftClickPatch, #647 fail-closed style, 8 sites). Plain left-click move-caret, double/triple-click word/line select, and the #647 right-click link-copy path are untouched. Copy on the reserved Ctrl/Cmd+C falls through to the widget's own clipboardTextSet — no copy patch needed (plan open-question 1 resolved).

Verification:

  • local zig build harness -Doptimize=Debug — green
  • zig build test-rich — exit 0 (baseline 4 font-fallback dvui_testing warnings pre-exist on clean main)
  • zig build test-rich-invariants — exit 0
  • zig fmt --check build.zig — clean
  • CI build-harness (wasm32-freestanding release) — green

Test strategy per plan row 6: operator smoke (goals 1–4) + build gates; the testing backend uses fallback fonts so byte-position hit-testing isn't reliable there. Living doc updated: docs/harness-limits.md (Keyboard & focus Shift+click row + Transcript copy/paste row). Escalated implementation documented in the single living planning comment on issue #752.

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #762

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/shift-click-select · 2 files · vendored TextLayoutWidget Shift+click select-extend (plan #752)
HEAD: 0ba43bd5aac8c7f4835e72c52c1690dc4526bc1e
Lenses run: L1, L3, L4, L6, L8, L9 (skip: L2 no secrets/API/workflows; L5 no poll/alloc/history change; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (in-canvas textEntry only; no DOM composer)

Attack assumed the patch was wrong. Needles 1–8 are unique against pin david-vanderson/dvui#4f810ef after the #647 right-click insert; the new sel_move variant is exhaustive at every switch (self.sel_move) (the bytesNeeded skip-site uses else). dvui.textEntry() is processEvents then draw/addText, so same-frame down_pt resolution works (existing click-to-caret already depends on that order). Web mousedown does not carry shiftKey, but Window.addEventMouseButton copies self.modifiers from the last key event — held Shift is visible as me.mod.shift(). Copy on reserved Ctrl/Cmd+C is already TextEntryWidget.copy()clipboardTextSet(sel). Feature-divide holds. build-harness path filter matches native/harness/** and is green.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L1 applyInvincibleShiftClickPatch needle 8 / TextLayoutWidget.processEvent — Shift+pointer press does not zero click_num. Release still hits the unpatched click_num += 1 arm (dragging() is null under dragPreStart until 3px). Composer: click at A (caret). Move to B (motion zeros click_num). Shift+click at B (range [A,B]; release sets click_num = 1). Plain click at B without moving → click_num == 1 at press → word-select, not caret-move + clear. That is the documented dismiss path (docs/harness-limits.md Keyboard row: “A plain (non-shift) click still moves the caret and clears the range”; plan #752 goal 3). Defender: “Shift+click is a click, so counting it toward double-click is stock dvui.” Fails: stock web does not count Shift+click toward double-click, and this PR’s own contract says the next plain click is caret-move. Fix is self.click_num = 0 on the shift press arm (and/or skip the increment on that release). high
Minor L1 selMovePre .shift_click sets start/end = min/max(sc.anchor, hit) with sc.anchor = selection.cursor at press. That is collapsed-caret only. dvui already has stock extend: selection.moveCursor(hit, true) (Shift+arrow / cursor_updown.select). (1) Click A, Shift+click B, Shift+click C → selection is [B,C], not [A,C]. (2) Drag-select [5,20] (cursor at 20), Shift+click 30 → [20,30], not [5,30]. Goal 1 (single Shift+click from a caret) still holds. Defender: “plan locked anchor = pre-press cursor and DoD is one Shift+click from a caret.” Survives as a real divergence from the PR’s “stock web behavior” claim and from the widget’s own Shift+arrow path. Not a DoD miss. high
Nit L8 docs/harness-limits.md Keyboard & focus Shift+click row dumps implementation internals (sel_move.shift_click, build.zig fail-closed patch, #647) into a user-facing limits table. A new hire reading chords has to parse vendored-patch archaeology to learn “Shift+click range-selects, Ctrl/C copies, plain click clears.” Defender: “living comment / implementer trail.” Limits tables are timeless product truth (AGENTS.md); the patch marker belongs in build.zig, which already has it. high

Residual risk

Operator smoke is the only behavioral proof (plan row 6; dvui_testing fallback fonts make hit-tests bogus). This attack did not run the canvas. If Shift keydown never reaches Window.addEventKey, me.mod.shift() is false and Shift+click silently degrades to a caret move — no user-visible error. A future dvui pin bump that edits any of the 8 needles panics the harness build (fail-closed, intended). Shift+click+drag does not track the pointer (motion only updates .mouse / .expand_pt); out of this slice.

Merge guidance

PASS WITH NOTES: safe to merge from this attack. The two Minors are cheap (click_num = 0 on the shift arm; moveCursor(hit, true) instead of min/max from cursor) and should land soon — they are not merge-blocking. Nit optional.

What was not attacked

Live DO runner beyond the already-green build-harness job; prod Gateway; in-browser operator smoke of composer / queue-row editor; child follow-up #753 (read-only textLayout Shift+click).

@btipling

Copy link
Copy Markdown
Owner Author

@_tmp/plan762-comment.md

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #762 (round 2)

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/shift-click-select · 2 files · vendored TextLayoutWidget Shift+click (plan #752)
HEAD: f30c62978e6a3590dadc20535180df9f3ce9e85d (round-1 follow-up on 0ba43bd)
Lenses run: L1, L3, L4, L6, L8, L9 (skip: L2 no secrets/API; L5 no poll/alloc; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (in-canvas textEntry only)

Round 1 (PASS WITH NOTES on 0ba43bd) asked for two Minors and a Nit. This head landed all three:

  • click_num: Shift+pointer press zeros click_num; release needle 9 is now pointer() and !me.mod.shift() so a Shift+click does not seed the next word/line select.
  • stock extend: selMovePre / lineBreak / addTextDone call selection.moveCursor(hit, true) — the same cursor == start ? move start : move end path as Shift+arrow — so A→B→C stays [A,C] and drag [5,20] + Shift+click 30 stays [5,30]. The “haven’t found it yet” scan copies cursor_updown.select (moveCursor(bytes_seen+end, true)), not a second min/max.
  • docs: Keyboard row dropped sel_move.shift_click / build.zig / #647 internals.

Needles 1–9 are unique against pin 4f810ef (click_num += 1 occurs once, so needle 9 is safe). #647 .right path is still outside the patched pointer() arm. Feature-divide holds. build-harness is green on this SHA.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Nit L1 Release gating keys off me.mod.shift() at mouseup, not “this press was a Shift+click”. Press already zeroed click_num and set .shift_click. Shift+mousedown (range set, click_num = 0), release Shift, then mouseup → !shift is true → click_num = 1. The next plain click is word-select, not caret-move + clear. Stock web counts the mousedown, not a shift-less mouseup. Fix: pointer() and self.sel_move != .shift_click (variant is still .shift_click at release; addText has not run yet in processEvent). Defender: “who releases Shift before the button?” Hyper-edge, and press-side zero still helps if they don’t. Not the documented dismiss path under a normal Shift+click. high

Residual risk

Operator smoke is still the only behavioral proof (plan row 6; dvui_testing fallback fonts). This attack did not run the canvas. Shift+click+drag does not track (press takes .shift_click, not .mouse.drag_pt) — out of slice. A future dvui pin that edits any of the 9 needles panics the harness build (fail-closed, intended). If Shift keydown never reaches Window.addEventKey, me.mod.shift() is false and Shift+click silently degrades to caret-move.

Merge guidance

PASS WITH NOTES. Round-1 Minors are fixed; this is safe to merge. The remaining Nit is a modifier-release race, not a drain/security/protocol hole — optional follow-up.

What was not attacked

Live DO runner beyond green build-harness; in-browser composer / queue-row smoke of repeated Shift+click and drag-then-Shift+click; child follow-up #753 (read-only textLayout).

@btipling

Copy link
Copy Markdown
Owner Author

/address_pr_feedback — round-2 Nit resolved (Zig only, no plan comment needed — single trivial localized fix handled in one turn, skill §0 simple-direct path).

Review: Round-2 adversarial review was PASS WITH NOTES with exactly one Nit (L1): the round-1 release gate if (me.button.pointer() and !me.mod.shift()) badged the click_num += 1 double-click counter off the live modifier at mouseup. A user who releases Shift before mouseup makes !me.mod.shift() true, so the counter reseeds to 1 and the next plain click would word-select instead of caret-move + clear — the documented dismiss path.

Fix (head a2f9826, one vendored-needle change): the release now gates on the press-state tag self.sel_move != .shift_click instead. The .shift_click variant persists through release (selMovePre zeroes down_pt but not the tag, and addText has not run yet within the processEvents batch), so this reliably means "this press was a Shift+click" regardless of when Shift is released — matching stock web (double-click counts mousedowns, not shift-less mouseups). Comment updated to state the intent.

Verification (authoritative, since Zig+tools/docs only → no TS/vitest gate): deleted the stale zig-pkg extract so the build re-extracted pristine dvui and genuinely re-applied Needle 9 (fail-closed count==1 passed; confirmed sel_move != .shift_click present at the release arm, no stale !me.mod.shift() remains). zig fmt --check clean · zig build test-rich exit 0 (the dvui_testing "failed command" lines are the documented pre-existing missing-font artifacts) · test-rich-invariants exit 0 · zig build harness -Doptimize=Debug exit 0 · CI build-harness (wasm32-freestanding release-wasm) run 32548253702 — SUCCESS on a2f9826.

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #762 (round 3)

Verdict: CONCERNS
Repo: btipling/invincible
Scope: mainplan/shift-click-select · 2 files · vendored TextLayoutWidget Shift+click (plan #752)
HEAD: a2f9826e2855de1d2854f4e8e55db20698f5b4c9 (round-2 Nit follow-up on f30c629)
Lenses run: L1, L3, L4, L6, L8 (skip: L2 no secrets; L5 no poll/alloc; L7 no clone; L9 no palette)
AGENTS.md read: yes · docs/feature-divide.md read (in-canvas textEntry only)

Round 2 (f30c629) had fixed the two Minors (stock moveCursor(hit, true); click_num = 0 on Shift+press) and gated release with pointer() and !me.mod.shift(). That skip survived a two-frame click because Window.modifiers still has Shift at mouseup. This head replaced that gate with self.sel_move != .shift_click to cover “let go of Shift before mouseup.” That substitution reopens the documented dismiss path on the common click.

Needles 1–9 still unique against pin 4f810ef. #647 .right path still outside the patched pointer() arm. build-harness green. Feature-divide holds. Stock-extend is unchanged and still correct.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L1 Needle 9 now skips the click_num += 1 only when self.sel_move == .shift_click. sel_move is frame-local. Pin 4f810ef deinit persists _sel_move_mouse_byte / _sel_move_expand_pt_* while captured — not .shift_click. Next-frame init reconstructs the widget with sel_move = .none. The patch comment (“addText has not run yet within this processEvents batch”) is only true when press and release are in the same event list. Web backend: JS add_event queues into Window, requestRender coalesces to one rAF. A human click is ~70–150 ms, so press is frame N and release is frame N+1. Frame N: Shift+press zeros click_num, sets .shift_click, deinit drops the variant (click_num == 0 also dataRemoves _click_num). Frame N+1: sel_move == .none!= .shift_clickclick_num = 1. Next plain click sees click_num == 1 at press → word-select, not caret-move + clear. That is plan #752 goal 3 / the Keyboard row. Round 2’s !me.mod.shift() skipped this increment on the common path (Shift still held). Round 3 keeps the hyper-edge (same-frame press+release, or Shift released before mouseup in a batched frame) and loses the common path. Fix: skip increment if self.sel_move == .shift_click or me.mod.shift() (union of both gates), or persist a captured _shift_click_press bool the way .mouse.byte is persisted. Defender: “dvui can deliver press+release in one frame.” True (README: low-FPS batching) — that is the fast path, not the typical web click. Defender: “press already zeros click_num.” Zero-at-press is undone by the unskipped release increment; that is exactly round 1’s Minor. high

Residual risk

Operator smoke is still the only behavioral proof (plan row 6). Shift+click+drag still does not track (press takes .shift_click, not .mouse.drag_pt) — out of slice. Pin drift on any of 9 needles panics the harness build (fail-closed, intended). If Shift keydown never reaches Window.addEventKey, me.mod.shift() is false at press and Shift+click degrades to caret-move — unchanged.

Merge guidance

CONCERNS: do not merge this SHA as a “Nit fix.” Round 2 (f30c629) is the safer head for the documented dismiss path. Restore a gate that survives a two-frame click (live Shift or persisted press-state). Not a drain/security/protocol hole — still a real contract break of goal 3.

What was not attacked

Live canvas click timing; child follow-up #753; prod Gateway.

…e OR live Shift (PR #762 review round-3 CONCERNS L1)

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #762 (round 4)

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/shift-click-select · 2 files · vendored TextLayoutWidget Shift+click (plan #752)
HEAD: f486aacf0f8ccaeff6b8d2645cc7f37b1a5e885d (round-3 CONCERNS follow-up on a2f9826)
Lenses run: L1, L3, L4, L6, L8 (skip: L2 no secrets; L5 no poll/alloc; L7 no clone; L9 no palette)
AGENTS.md read: yes · docs/feature-divide.md read (in-canvas textEntry only)

Round 3 (a2f9826) gated release on sel_move != .shift_click only and reopened goal 3 on a two-frame click. This head ORs the live modifier back in:

pointer() and self.sel_move != .shift_click and !me.mod.shift()

Skip increment when either same-frame press-state is .shift_click or Shift is still held at mouseup. That is the union this attack asked for.

Path sel_move at release Shift at mouseup Increment?
Same-frame Shift+click, Shift held .shift_click yes no
Same-frame Shift+click, Shift released first .shift_click no no
Two-frame Shift+click, Shift held (common) .none yes no
Plain click (double-click counting) .mouse (persisted) no yes

Needles 1–9 unique against pin 4f810ef. Stock moveCursor(hit, true) unchanged. #647 .right path still outside the patched pointer() arm. click_num = 0 still on Shift+press. Docs still product-facing. Feature-divide holds. build-harness green on this SHA.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Nit L1 The union still does not cover two-frame + Shift released before mouseup. sel_move is not persisted (only .mouse / .expand_pt while captured), so frame N+1 is .none and !shiftclick_num = 1. Shift+mousedown (frame N), release Shift, mouseup (frame N+1). Next plain click word-selects. Persist _shift_click_press while captured (the other round-3 option) would close it. Defender: “that is the hyper-edge the Nit named, and the common path (Shift held through the click) is restored.” Survives: not goal 3 under a normal Shift+click. Optional persist, not a merge block. high

Residual risk

Operator smoke is still the only behavioral proof (plan row 6; dvui_testing fallback fonts). Shift+click+drag does not track. Pin drift on any of 9 needles panics the harness build (fail-closed, intended). If Shift keydown never reaches Window.addEventKey, me.mod.shift() is false at press and Shift+click degrades to caret-move.

Merge guidance

PASS WITH NOTES. Round-3 CONCERNS is fixed; this is safe to merge. The leftover Nit is the two-frame modifier-release race — optional persist flag, not a drain/security/protocol hole.

What was not attacked

Live canvas click timing; child follow-up #753; prod Gateway.

@btipling

Copy link
Copy Markdown
Owner Author

Round-4 Nit resolved — persisted _shift_click_press flag (Zig only; single localized fix → skill §0 simple-direct path, no planning comment; appended here for the review trail)

Review: Round-4 adversarial review was PASS WITH NOTES with exactly one Nit (L1): the union release gate
self.sel_move != .shift_click and !me.mod.shift() still missed a two-frame click where Shift is released
before mouseup
. sel_move is frame-local — the .shift_click variant re-inits to .none every frame — so on
frame N+1 (release) both sel_move == .shift_click (false) and live Shift (false) hold, and the release
click_num += 1 fires. The next plain click would word-select instead of caret-move + clear — the documented
dismiss path (plan #752 goal 3). The reviewer named the remedy explicitly: "persist a captured _shift_click_press
bool the way .mouse.byte is persisted."

Fix (head f58ba90, build.zig applyInvincibleShiftClickPatch — needles 1–9 unchanged):

  • Needle 8 (press): the Shift+press arm now dvui.dataSet(null, self.data().id, "_shift_click_press", true)
    in addition to zeroing click_num — a store flag that crosses the press→release frame gap, exactly the way
    .mouse.byte is persisted while captured.
  • Needle 9 (release): the gate is now a three-way union —
    self.sel_move == .shift_click (same-frame press+release) or me.mod.shift() (common two-frame click,
    Shift held) or the persisted _shift_click_press flag (two-frame + Shift released before mouseup). The flag
    is dataRemoved here so it is consumed once and never suppresses a later plain click's double-click count.
Path sel_move @ release Shift @ mouseup _shift_click_press Increment?
Same-frame Shift+click .shift_click any set no
Two-frame Shift+click, Shift held .none yes set no
Two-frame, Shift released before mouseup (round-4 race) .none no set no
Plain click (double-click counting) .mouse (persisted) no absent yes

The common path and prior rounds are unchanged; this only closes the reviewer's last named race by persisting
the press state across capture frames (mirroring the existing .mouse.byte mechanism).

Verification (authoritative, since Zig-only → no TS/vitest gate): deleted the stale zig-pkg extract so the
build re-extracted pristine dvui and genuinely re-applied needles 1–9 (fail-closed count==1 passed — the
build would have panicked otherwise), then confirmed the new _shift_click_press set/read/remove is present at the
patched sites. zig fmt --check clean · zig build test-rich exit 0 (the 4 dvui_testing "failed command"
lines are the documented pre-existing fallback-font artifacts: composer_layout, model_picker_layout, paint_diff,
transcript_split_layout — bytes-identical on clean main) · test-rich-invariants exit 0 ·
zig build harness -Doptimize=Debug exit 0 (full Wasm compile) · CI build-harness (wasm32-freestanding
release-wasm) run 32552445588 — SUCCESS on f58ba90.
PR mergeStateStatus: CLEAN.

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #762 (round 5)

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/shift-click-select · 2 files · vendored TextLayoutWidget Shift+click (plan #752)
HEAD: f58ba906254573319673c856580f4a0b5ba269c1 (round-4 Nit follow-up on f486aac)
Lenses run: L1, L3, L4, L6, L8 (skip: L2 no secrets; L5 no poll/alloc; L7 no clone; L9 no palette)
AGENTS.md read: yes · docs/feature-divide.md read (in-canvas textEntry only)

Round 4 (PASS WITH NOTES on f486aac) left the two-frame + Shift-released-before-mouseup hole. This head persists _shift_click_press at Shift+press (dataSet, same store as .mouse.byte) and three-way-ORs it at release:

shift_click_release =
    sel_move == .shift_click      // (a) same-frame
    or me.mod.shift()             // (b) two-frame, Shift held
    or dataGet(_shift_click_press) // (c) two-frame, Shift already up
dataRemove(_shift_click_press)
increment iff pointer and !shift_click_release
Path Increment?
Same-frame Shift+click no (a)
Two-frame, Shift held no (b)
Two-frame, Shift released before mouseup no (c)
Plain click (double-click counting) yes (no flag)

Needles 1–9 unique against pin 4f810ef. Stock moveCursor(hit, true) unchanged. #647 .right path still outside the patched pointer() arm. click_num = 0 still on Shift+press. Docs still product-facing. Feature-divide holds. build-harness green on this SHA.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Nit L1 dataRemove("_shift_click_press") sits inside the unpatched dragging() == null click-without-drag arm. A Shift+press that becomes a drag never consumes the flag. Shift+mousedown, move ≥3px, mouseup: release skips the needle. Flag stays. Next plain click-without-drag: shift_click_release is true → skip click_num += 1 once, then consume. Fail-closed (won’t word-select extra); one click of a later double-click sequence is ignored. Out-of-slice for Shift+click+drag tracking, leftover of the persist. Fix: dataRemove on any pointer release (or in deinit when !captured). Defender: “drag is out of slice; fail-closed.” Survives as a one-click counter skip, not goal 3. high

Residual risk

Operator smoke is still the only behavioral proof (plan row 6; dvui_testing fallback fonts). Shift+click+drag does not track. Pin drift on any of 9 needles panics the harness build (fail-closed, intended). If Shift keydown never reaches Window.addEventKey, me.mod.shift() is false at press and Shift+click degrades to caret-move.

Merge guidance

PASS WITH NOTES. Round-4 Nit is closed; this is safe to merge. The leftover is a fail-closed stale-flag after Shift+drag, not a drain/security/protocol hole.

What was not attacked

Live canvas click timing; child follow-up #753; prod Gateway.

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #762 (round 6)

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/shift-click-select · 2 files · vendored TextLayoutWidget Shift+click (plan #752)
HEAD: 070446c8bb03aff45b78a41d23ec05f30276ca4a (round-5 Nit follow-up on f58ba90)
Lenses run: L1, L3, L4, L6, L8 (skip: L2 no secrets; L5 no poll/alloc; L7 no clone; L9 no palette)
AGENTS.md read: yes · docs/feature-divide.md read (in-canvas textEntry only)

Round 5 left _shift_click_press consumed only inside dragging() == null. This head adds needle 10: dataRemove on any captured pointer/middle release, immediately before captureMouse(null) / dragEnd(). Needle 9 still reads the flag for the increment skip; needle 10 always consumes it (no-op if absent). build-harness green proves needles 1–10 each match exactly once against pin 4f810ef after the #647 insert.

Path Increment? Flag
Same-frame Shift+click no (a) consume at 10
Two-frame, Shift held no (b) consume at 10
Two-frame, Shift released before mouseup no (c) consume at 10
Shift+press then drag ≥3px n/a (drag arm) consume at 10
Plain click yes no flag

Stock moveCursor(hit, true) unchanged. #647 .right path still outside the patched pointer() arm. click_num = 0 still on Shift+press. Docs still product-facing. Feature-divide holds.

Findings

No remaining findings from this attack. Round-5 Nit is fixed; earlier Minors stay fixed.

Residual risk

Operator smoke is still the only behavioral proof (plan row 6; dvui_testing fallback fonts). Shift+click+drag does not track (press takes .shift_click, not .mouse.drag_pt) — out of slice; the flag is now consumed on that path anyway. Pin drift on any of 10 needles panics the harness build (fail-closed, intended). If Shift keydown never reaches Window.addEventKey, me.mod.shift() is false at press and Shift+click degrades to caret-move. A capture-stolen-without-release still leaves the flag until the next captured release on this widget (fail-closed one-click skip) — not a live composer path.

Merge guidance

PASS WITH NOTES. Safe to merge from this attack. Residual is operator smoke / pin-drift, not a drain/security/protocol hole.

What was not attacked

Live canvas click timing; child follow-up #753; prod Gateway.

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.

plan: harness — Shift+click extends text selection in composer + queue-row editor (#749)

1 participant