Snapshot authorship, click targeting, and shares without a file list - #17
Draft
angusbezzina wants to merge 3 commits into
Draft
Snapshot authorship, click targeting, and shares without a file list#17angusbezzina wants to merge 3 commits into
angusbezzina wants to merge 3 commits into
Conversation
scripts/test-e2e.sh has been exiting 1 on two Suite 2 assertions. Neither was the app and neither was quite the test. __resolve matches every element whose trimmed text equals the target, which for one sidebar row is the <li>, the <button> inside it, and the <span> inside that. Click took els[0] — document order — so it clicked the <li>, an element with no handler. The interaction reported ok and the document never moved. That is every text= click in the tree, not just this suite's. It also explains why only one of the two navigation cases failed. Neither click was working; the second case happened to assert the heading of a file that was already open, so it passed vacuously. Fixing the targeting is what makes it a real assertion. Click now prefers the innermost INTERACTIVE match, falling back to the innermost match of any kind, whose click still bubbles to whatever handler sits above it. The breadcrumb case asserted a control the native app does not have. web/src/lib/PathBreadcrumb.svelte is imported by nothing — only a stale rule in the hosted stylesheet mentions it — and a live window reports zero matches for [class*=breadcrumb], nav[aria-label] and [data-slot*=breadcrumb]. The selector could only ever find nothing, so the case asserted the absence of a control rather than any behaviour. It now asserts what it was reaching for, against something that exists: the sidebar marks the open row data-active="true" and carries its full path. Both navigations now wait for the heading to BECOME what they expect. Waiting for `h1` cannot do that — the previous document's h1 already satisfies it, so the wait returned instantly and the fixed sleep after it raced the navigation. Fixes attn-537h.
attn-udu8 publishes the images a document references, but only for a share carrying an explicit selection. Without one, selected_share_wire_path returned None, publish_asset_snapshot failed for want of a portable name, and every image was scanned, approved, and then dropped with a log line. The reviewer saw placeholder cards and no frontend change could have helped — the bytes were never sent. That is the CLI path: `attn review share <path>`, and the legacy share(path) wrapper that passes an empty selection. It stayed invisible because the native Share dialog always sends a selection, so every live verification of attn-udu8 went through the one path that worked. A share with no curated list still has a root, and every file it publishes still needs a portable name. Wire paths now derive from that root — the shared directory, or a shared file's own parent — with membership of the curated list authorising the name when there is one, and containment in the root when there is not. Nothing outside the root becomes nameable either way. Verified against a live relay: `attn review share tests/fixtures/images.md` publishes all three assets where it previously published none. Fixes attn-x2zq.
attn-lb7p made workspace manifests prove authorship. Doing it exposed that the manifest was the only place in the snapshot path where authorship was checked at all. Every other snapshot — the documents a reviewer reads, and the image assets attn-udu8 added — was accepted on a signature from ANY device in the room directory, because that is all InboundPipeline establishes. A reviewer holding a comment- or suggest-tier grant is such a device. Nothing stopped one minting a SnapshotCreated for an arbitrary fileId and having every other reviewer render it as the owner's document, or as an image inside it. Snapshots are structurally owner-published: both republish paths go through find_room_for_path, which needs a local share record that only the sharing machine has. A reviewer cannot legitimately publish one, so requiring the owner's signature refuses exactly the events that had no business existing. Establishing that first is why this is safe — a wrong tightening here does not degrade an image, it stops the shared document rendering. The pinned-key verification is now one helper shared with the manifest path rather than two copies of the same crypto. Rooms with no pinned key — v2, and v3 rooms joined before invites carried one — keep behaving exactly as they did: they have no way to establish authorship and must not lose their documents to a check they cannot satisfy. Verified live: a real share still renders all four images with zero hydration rejections. The two forgery tests are mutation-checked — neutering the check kills them and nothing else. Fixes attn-1n67.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three follow-ups surfaced while shipping the image work, each filed after v0.10.0 and each fixed here. Draft because the third one changes what a reviewer will accept, and that deserves a careful read.
Closes
attn-1n67,attn-537h,attn-x2zq.1. Every snapshot has to say who published it (
attn-1n67, P1)attn-lb7pmade workspace manifests prove authorship. Doing it exposed that the manifest was the only place in the snapshot path where authorship was checked at all.Every other snapshot — the documents a reviewer reads, and the image assets
attn-udu8added — was accepted on a signature from any device in the room directory, because that is allInboundPipelineestablishes. A reviewer holding a comment- or suggest-tier grant is such a device. Nothing stopped one minting aSnapshotCreatedfor an arbitraryfileIdand having every other reviewer render it as the owner's document, or as an image inside it.Why it is safe to refuse them. Snapshots are structurally owner-published: both republish paths go through
find_room_for_path, which needs a local share record that only the sharing machine has. A reviewer cannot legitimately publish one. Establishing that before tightening is the whole risk management here — a wrong tightening does not degrade an image, it stops the shared document rendering.Rooms with no pinned key (v2, and v3 rooms joined before invites carried one) behave exactly as before. They cannot establish authorship and must not lose their documents to a check they cannot satisfy.
The pinned-key verification is now one helper shared with the manifest path rather than two copies of the same crypto.
2. A
text=click lands on the thing you would have clicked (attn-537h)scripts/test-e2e.shhad been exiting 1 on two Suite 2 assertions. Neither was the app and neither was quite the test.__resolvematches every element whose trimmed text equals the target — for one sidebar row that is the<li>, the<button>inside it, and the<span>inside that.Clicktookels[0], document order, so it clicked the<li>: an element with no handler. The interaction reportedokand the document never moved. That is everytext=click in the tree, not just this suite's.It also explains why only one of the two navigation cases failed. Neither click worked; the second happened to assert the heading of a file that was already open, so it passed vacuously. Fixing the targeting is what makes it a real assertion.
The breadcrumb case asserted a control the native app does not have —
PathBreadcrumb.svelteis imported by nothing, and a live window reports zero matches for[class*=breadcrumb],nav[aria-label]and[data-slot*=breadcrumb]. It now asserts what it was reaching for, against something that exists.3. A share without a curated file list still carries its images (
attn-x2zq)attn-udu8publishes referenced images, but only for a share carrying an explicit selection. Without oneselected_share_wire_pathreturnedNone,publish_asset_snapshotfailed for want of a portable name, and every image was scanned, approved, and then dropped with a log line.That is the CLI path —
attn review share <path>, and the legacyshare(path)wrapper. It stayed invisible because the native Share dialog always sends a selection, so every live verification of attn-udu8 went through the one path that worked.Wire paths now derive from the share's own root, with list membership authorising the name when there is a list and containment in the root when there is not. Nothing outside the root becomes nameable either way.
Verification
attn review share tests/fixtures/images.mdnow publishes all three assets where it previously published none.scripts/test-e2e.shexits 0 — 26 pass, 0 fail — for the first time in a long while.Gates
cargo fmt --check·clippy --all-targets -D warnings· 593 + 706 Rust tests · 137 web test files · typecheck · route-bundle gate.What to look hardest at
Item 1. The argument that no legitimate non-owner snapshot exists rests on both republish paths needing a local share record. If there is a flow I did not find — an agent, or something in live collab — the symptom is a shared document that refuses to render rather than anything subtle. The fallback for unpinned rooms limits it to v3 rooms joined since
attn-lb7p.Not addressed here
attn-7xl.7.4rather than filed again.